* [PATCH 0/3] Introduce devm_clk_register()
@ 2012-09-19 6:05 Stephen Boyd
2012-09-19 6:05 ` [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Stephen Boyd @ 2012-09-19 6:05 UTC (permalink / raw)
To: linux-arm-kernel
The first patch in this series fixes error checking in the wm831x clock
driver and is here to prevent context conflicts in the third patch.
I split it out in case it needed to merge sooner rather than later.
The goal of this series is to add devm_clk_register() so I can use it in
some MSM clock code I'm sending out in the near future. The second
patch adds the API and the third patch moves over an existing user of
clk_unregister() to the devm API.
Stephen Boyd (3):
clk: wm831x: Fix clk_register() error code checking
clk: Add devm_clk_{register,unregister}()
clk: wm831x: Use devm_clk_register() to simplify code
drivers/clk/clk-wm831x.c | 34 ++++---------
drivers/clk/clk.c | 111 +++++++++++++++++++++++++++++++++++--------
include/linux/clk-provider.h | 2 +
3 files changed, 101 insertions(+), 46 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking 2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd @ 2012-09-19 6:05 ` Stephen Boyd 2012-09-20 1:31 ` Mark Brown 2012-09-19 6:05 ` [PATCH 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd ` (3 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Stephen Boyd @ 2012-09-19 6:05 UTC (permalink / raw) To: linux-arm-kernel clk_register() returns an ERR_PTR upon failure, not NULL. Fix these error paths. Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk-wm831x.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c index e7b7765..eb1afaf 100644 --- a/drivers/clk/clk-wm831x.c +++ b/drivers/clk/clk-wm831x.c @@ -371,20 +371,20 @@ static __devinit int wm831x_clk_probe(struct platform_device *pdev) clkdata->xtal_hw.init = &wm831x_xtal_init; clkdata->xtal = clk_register(&pdev->dev, &clkdata->xtal_hw); - if (!clkdata->xtal) - return -EINVAL; + if (IS_ERR(clkdata->xtal)) + return PTR_ERR(clkdata->xtal); clkdata->fll_hw.init = &wm831x_fll_init; clkdata->fll = clk_register(&pdev->dev, &clkdata->fll_hw); - if (!clkdata->fll) { - ret = -EINVAL; + if (IS_ERR(clkdata->fll)) { + ret = PTR_ERR(clkdata->fll); goto err_xtal; } clkdata->clkout_hw.init = &wm831x_clkout_init; clkdata->clkout = clk_register(&pdev->dev, &clkdata->clkout_hw); - if (!clkdata->clkout) { - ret = -EINVAL; + if (IS_ERR(clkdata->clkout)) { + ret = PTR_ERR(clkdata->clkout); goto err_fll; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking 2012-09-19 6:05 ` [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd @ 2012-09-20 1:31 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2012-09-20 1:31 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 18, 2012 at 11:05:28PM -0700, Stephen Boyd wrote: > clk_register() returns an ERR_PTR upon failure, not NULL. Fix > these error paths. Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> this is an API change since the driver was sent which the driver didn't get updatd for. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] clk: Add devm_clk_{register,unregister}() 2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd 2012-09-19 6:05 ` [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd @ 2012-09-19 6:05 ` Stephen Boyd 2012-09-21 2:33 ` Stephen Boyd 2012-09-19 6:05 ` [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd ` (2 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Stephen Boyd @ 2012-09-19 6:05 UTC (permalink / raw) To: linux-arm-kernel Some clock drivers can be simplified if devres takes care of unregistering any registered clocks along error paths. Introduce devm_clk_register() so that clock drivers get unregistration for free along with simplified error paths. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk.c | 111 +++++++++++++++++++++++++++++++++++-------- include/linux/clk-provider.h | 2 + 2 files changed, 92 insertions(+), 21 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 56e4495e..6852809 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -17,6 +17,7 @@ #include <linux/list.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/device.h> static DEFINE_SPINLOCK(enable_lock); static DEFINE_MUTEX(prepare_lock); @@ -1361,28 +1362,9 @@ struct clk *__clk_register(struct device *dev, struct clk_hw *hw) } EXPORT_SYMBOL_GPL(__clk_register); -/** - * clk_register - allocate a new clock, register it and return an opaque cookie - * @dev: device that is registering this clock - * @hw: link to hardware-specific clock data - * - * clk_register is the primary interface for populating the clock tree with new - * clock nodes. It returns a pointer to the newly allocated struct clk which - * cannot be dereferenced by driver code but may be used in conjuction with the - * rest of the clock API. In the event of an error clk_register will return an - * error code; drivers must test for an error code after calling clk_register. - */ -struct clk *clk_register(struct device *dev, struct clk_hw *hw) +static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk) { int i, ret; - struct clk *clk; - - clk = kzalloc(sizeof(*clk), GFP_KERNEL); - if (!clk) { - pr_err("%s: could not allocate clk\n", __func__); - ret = -ENOMEM; - goto fail_out; - } clk->name = kstrdup(hw->init->name, GFP_KERNEL); if (!clk->name) { @@ -1420,7 +1402,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw) ret = __clk_init(dev, clk); if (!ret) - return clk; + return 0; fail_parent_names_copy: while (--i >= 0) @@ -1429,6 +1411,36 @@ fail_parent_names_copy: fail_parent_names: kfree(clk->name); fail_name: + return ret; +} + +/** + * clk_register - allocate a new clock, register it and return an opaque cookie + * @dev: device that is registering this clock + * @hw: link to hardware-specific clock data + * + * clk_register is the primary interface for populating the clock tree with new + * clock nodes. It returns a pointer to the newly allocated struct clk which + * cannot be dereferenced by driver code but may be used in conjuction with the + * rest of the clock API. In the event of an error clk_register will return an + * error code; drivers must test for an error code after calling clk_register. + */ +struct clk *clk_register(struct device *dev, struct clk_hw *hw) +{ + int ret; + struct clk *clk; + + clk = kzalloc(sizeof(*clk), GFP_KERNEL); + if (!clk) { + pr_err("%s: could not allocate clk\n", __func__); + ret = -ENOMEM; + goto fail_out; + } + + ret = _clk_register(dev, hw, clk); + if (!ret) + return clk; + kfree(clk); fail_out: return ERR_PTR(ret); @@ -1444,6 +1456,63 @@ EXPORT_SYMBOL_GPL(clk_register); void clk_unregister(struct clk *clk) {} EXPORT_SYMBOL_GPL(clk_unregister); +static void devm_clk_release(struct device *dev, void *res) +{ + clk_unregister(res); +} + +/** + * devm_clk_register - resource managed clk_register() + * @dev: device that is registering this clock + * @hw: link to hardware-specific clock data + * + * Managed clk_register(). Clocks returned from this function are + * automatically clk_unregister()ed on driver detach. See clk_register() for + * more information. + */ +struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw) +{ + struct clk *clk; + int ret; + + clk = devres_alloc(devm_clk_release, sizeof(*clk), GFP_KERNEL); + if (!clk) + return ERR_PTR(-ENOMEM); + + ret = _clk_register(dev, hw, clk); + if (!ret) { + devres_add(dev, clk); + } else { + devres_free(clk); + clk = ERR_PTR(ret); + } + + return clk; +} +EXPORT_SYMBOL_GPL(devm_clk_register); + +static int devm_clk_match(struct device *dev, void *res, void *data) +{ + struct clk *c = res; + if (WARN_ON(!c)) + return 0; + return c == data; +} + +/** + * devm_clk_unregister - resource managed clk_unregister() + * @clk: clock to unregister + * + * Deallocate a clock allocated with devm_clk_register(). Normally + * this function will not need to be called and the resource management + * code will ensure that the resource is freed. + */ +void devm_clk_unregister(struct device *dev, struct clk *clk) +{ + WARN_ON(devres_destroy(dev, devm_clk_release, devm_clk_match, clk)); +} +EXPORT_SYMBOL(devm_clk_unregister); + /*** clk rate change notifiers ***/ /** diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index c127315..710c6cb 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -327,8 +327,10 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, * error code; drivers must test for an error code after calling clk_register. */ struct clk *clk_register(struct device *dev, struct clk_hw *hw); +struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw); void clk_unregister(struct clk *clk); +void devm_clk_unregister(struct device *dev, struct clk *clk); /* helper functions */ const char *__clk_get_name(struct clk *clk); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] clk: Add devm_clk_{register,unregister}() 2012-09-19 6:05 ` [PATCH 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd @ 2012-09-21 2:33 ` Stephen Boyd 2012-09-22 1:07 ` Mike Turquette 0 siblings, 1 reply; 16+ messages in thread From: Stephen Boyd @ 2012-09-21 2:33 UTC (permalink / raw) To: linux-arm-kernel On 09/18/12 23:05, Stephen Boyd wrote: > +void devm_clk_unregister(struct device *dev, struct clk *clk) > +{ > + WARN_ON(devres_destroy(dev, devm_clk_release, devm_clk_match, clk)); Hm... I guess this needs to be devres_release() instead of destroy. Can you squash this in or should I resend for the few character change? diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 6852809..f02f4fe 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1509,7 +1509,7 @@ static int devm_clk_match(struct device *dev, void *res, void *data) */ void devm_clk_unregister(struct device *dev, struct clk *clk) { - WARN_ON(devres_destroy(dev, devm_clk_release, devm_clk_match, clk)); + WARN_ON(devres_release(dev, devm_clk_release, devm_clk_match, clk)); } EXPORT_SYMBOL(devm_clk_unregister); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] clk: Add devm_clk_{register,unregister}() 2012-09-21 2:33 ` Stephen Boyd @ 2012-09-22 1:07 ` Mike Turquette 2012-09-22 7:20 ` Stephen Boyd 0 siblings, 1 reply; 16+ messages in thread From: Mike Turquette @ 2012-09-22 1:07 UTC (permalink / raw) To: linux-arm-kernel Quoting Stephen Boyd (2012-09-20 19:33:42) > On 09/18/12 23:05, Stephen Boyd wrote: > > +void devm_clk_unregister(struct device *dev, struct clk *clk) > > +{ > > + WARN_ON(devres_destroy(dev, devm_clk_release, devm_clk_match, clk)); > > Hm... I guess this needs to be devres_release() instead of destroy. Can > you squash this in or should I resend for the few character change? > I'm not taking any more changes for 3.7, so in the interest of me being lazy can you resend with the fixup? Thanks, Mike > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 6852809..f02f4fe 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1509,7 +1509,7 @@ static int devm_clk_match(struct device *dev, void *res, void *data) > */ > void devm_clk_unregister(struct device *dev, struct clk *clk) > { > - WARN_ON(devres_destroy(dev, devm_clk_release, devm_clk_match, clk)); > + WARN_ON(devres_release(dev, devm_clk_release, devm_clk_match, clk)); > } > EXPORT_SYMBOL(devm_clk_unregister); > > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] clk: Add devm_clk_{register,unregister}() 2012-09-22 1:07 ` Mike Turquette @ 2012-09-22 7:20 ` Stephen Boyd 0 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2012-09-22 7:20 UTC (permalink / raw) To: linux-arm-kernel On 9/21/2012 6:07 PM, Mike Turquette wrote > I'm not taking any more changes for 3.7, so in the interest of me being > lazy can you resend with the fixup? Sure. I'll pick up Mark's acks too and send the whole series again. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code 2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd 2012-09-19 6:05 ` [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd 2012-09-19 6:05 ` [PATCH 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd @ 2012-09-19 6:05 ` Stephen Boyd 2012-09-20 1:40 ` Mark Brown 2012-09-22 10:06 ` [PATCH 0/3] Introduce devm_clk_register() Russell King - ARM Linux 2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd 4 siblings, 1 reply; 16+ messages in thread From: Stephen Boyd @ 2012-09-19 6:05 UTC (permalink / raw) To: linux-arm-kernel Move this driver to use devm_clk_register() to simplify some error paths and reduce lines of code. Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk-wm831x.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c index eb1afaf..db4fbf2 100644 --- a/drivers/clk/clk-wm831x.c +++ b/drivers/clk/clk-wm831x.c @@ -370,43 +370,27 @@ static __devinit int wm831x_clk_probe(struct platform_device *pdev) clkdata->xtal_ena = ret & WM831X_XTAL_ENA; clkdata->xtal_hw.init = &wm831x_xtal_init; - clkdata->xtal = clk_register(&pdev->dev, &clkdata->xtal_hw); + clkdata->xtal = devm_clk_register(&pdev->dev, &clkdata->xtal_hw); if (IS_ERR(clkdata->xtal)) return PTR_ERR(clkdata->xtal); clkdata->fll_hw.init = &wm831x_fll_init; - clkdata->fll = clk_register(&pdev->dev, &clkdata->fll_hw); - if (IS_ERR(clkdata->fll)) { - ret = PTR_ERR(clkdata->fll); - goto err_xtal; - } + clkdata->fll = devm_clk_register(&pdev->dev, &clkdata->fll_hw); + if (IS_ERR(clkdata->fll)) + return PTR_ERR(clkdata->fll); clkdata->clkout_hw.init = &wm831x_clkout_init; - clkdata->clkout = clk_register(&pdev->dev, &clkdata->clkout_hw); - if (IS_ERR(clkdata->clkout)) { - ret = PTR_ERR(clkdata->clkout); - goto err_fll; - } + clkdata->clkout = devm_clk_register(&pdev->dev, &clkdata->clkout_hw); + if (IS_ERR(clkdata->clkout)) + return PTR_ERR(clkdata->clkout); dev_set_drvdata(&pdev->dev, clkdata); return 0; - -err_fll: - clk_unregister(clkdata->fll); -err_xtal: - clk_unregister(clkdata->xtal); - return ret; } static int __devexit wm831x_clk_remove(struct platform_device *pdev) { - struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); - - clk_unregister(clkdata->clkout); - clk_unregister(clkdata->fll); - clk_unregister(clkdata->xtal); - return 0; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code 2012-09-19 6:05 ` [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd @ 2012-09-20 1:40 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2012-09-20 1:40 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 18, 2012 at 11:05:30PM -0700, Stephen Boyd wrote: > Move this driver to use devm_clk_register() to simplify some > error paths and reduce lines of code. Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] Introduce devm_clk_register() 2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd ` (2 preceding siblings ...) 2012-09-19 6:05 ` [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd @ 2012-09-22 10:06 ` Russell King - ARM Linux 2012-09-24 20:20 ` Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd 4 siblings, 1 reply; 16+ messages in thread From: Russell King - ARM Linux @ 2012-09-22 10:06 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 18, 2012 at 11:05:27PM -0700, Stephen Boyd wrote: > The first patch in this series fixes error checking in the wm831x clock > driver and is here to prevent context conflicts in the third patch. > I split it out in case it needed to merge sooner rather than later. > > The goal of this series is to add devm_clk_register() so I can use it in > some MSM clock code I'm sending out in the near future. The second > patch adds the API and the third patch moves over an existing user of > clk_unregister() to the devm API. Can we guarantee that the clocks are unused when the module is removed? If we can't make that guarantee, then devm_* should not be used here, and instead there should be refcounting done in the clocks (that's what the __clk_get() and __clk_put() hooks are there for.) ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] Introduce devm_clk_register() 2012-09-22 10:06 ` [PATCH 0/3] Introduce devm_clk_register() Russell King - ARM Linux @ 2012-09-24 20:20 ` Stephen Boyd 2012-09-24 20:35 ` Russell King - ARM Linux 0 siblings, 1 reply; 16+ messages in thread From: Stephen Boyd @ 2012-09-24 20:20 UTC (permalink / raw) To: linux-arm-kernel On 09/22/12 03:06, Russell King - ARM Linux wrote: > On Tue, Sep 18, 2012 at 11:05:27PM -0700, Stephen Boyd wrote: >> The first patch in this series fixes error checking in the wm831x clock >> driver and is here to prevent context conflicts in the third patch. >> I split it out in case it needed to merge sooner rather than later. >> >> The goal of this series is to add devm_clk_register() so I can use it in >> some MSM clock code I'm sending out in the near future. The second >> patch adds the API and the third patch moves over an existing user of >> clk_unregister() to the devm API. > Can we guarantee that the clocks are unused when the module is removed? > If we can't make that guarantee, then devm_* should not be used here, > and instead there should be refcounting done in the clocks (that's what > the __clk_get() and __clk_put() hooks are there for.) We could guarantee that when clk_unregister() is actually implemented. __clk_get() would need to forward a call to the module providing the clock via try_module_get(). Similarly we would call module_put() in __clk_put(). That would prevent unbinding the driver from the device via module removal. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] Introduce devm_clk_register() 2012-09-24 20:20 ` Stephen Boyd @ 2012-09-24 20:35 ` Russell King - ARM Linux 0 siblings, 0 replies; 16+ messages in thread From: Russell King - ARM Linux @ 2012-09-24 20:35 UTC (permalink / raw) To: linux-arm-kernel On Mon, Sep 24, 2012 at 01:20:42PM -0700, Stephen Boyd wrote: > On 09/22/12 03:06, Russell King - ARM Linux wrote: > > On Tue, Sep 18, 2012 at 11:05:27PM -0700, Stephen Boyd wrote: > >> The first patch in this series fixes error checking in the wm831x clock > >> driver and is here to prevent context conflicts in the third patch. > >> I split it out in case it needed to merge sooner rather than later. > >> > >> The goal of this series is to add devm_clk_register() so I can use it in > >> some MSM clock code I'm sending out in the near future. The second > >> patch adds the API and the third patch moves over an existing user of > >> clk_unregister() to the devm API. > > Can we guarantee that the clocks are unused when the module is removed? > > If we can't make that guarantee, then devm_* should not be used here, > > and instead there should be refcounting done in the clocks (that's what > > the __clk_get() and __clk_put() hooks are there for.) > > We could guarantee that when clk_unregister() is actually implemented. > __clk_get() would need to forward a call to the module providing the > clock via try_module_get(). Similarly we would call module_put() in > __clk_put(). That would prevent unbinding the driver from the device via > module removal. Strangely enough... mach-integrator's clkdev.h... precisely because it has a module which may provide a clock. static inline int __clk_get(struct clk *clk) { return try_module_get(clk->owner); } static inline void __clk_put(struct clk *clk) { module_put(clk->owner); } ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking 2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd ` (3 preceding siblings ...) 2012-09-22 10:06 ` [PATCH 0/3] Introduce devm_clk_register() Russell King - ARM Linux @ 2012-09-24 20:38 ` Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd ` (2 more replies) 4 siblings, 3 replies; 16+ messages in thread From: Stephen Boyd @ 2012-09-24 20:38 UTC (permalink / raw) To: linux-arm-kernel clk_register() returns an ERR_PTR upon failure, not NULL. Fix these error paths. Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- v2: No changes drivers/clk/clk-wm831x.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c index e7b7765..eb1afaf 100644 --- a/drivers/clk/clk-wm831x.c +++ b/drivers/clk/clk-wm831x.c @@ -371,20 +371,20 @@ static __devinit int wm831x_clk_probe(struct platform_device *pdev) clkdata->xtal_hw.init = &wm831x_xtal_init; clkdata->xtal = clk_register(&pdev->dev, &clkdata->xtal_hw); - if (!clkdata->xtal) - return -EINVAL; + if (IS_ERR(clkdata->xtal)) + return PTR_ERR(clkdata->xtal); clkdata->fll_hw.init = &wm831x_fll_init; clkdata->fll = clk_register(&pdev->dev, &clkdata->fll_hw); - if (!clkdata->fll) { - ret = -EINVAL; + if (IS_ERR(clkdata->fll)) { + ret = PTR_ERR(clkdata->fll); goto err_xtal; } clkdata->clkout_hw.init = &wm831x_clkout_init; clkdata->clkout = clk_register(&pdev->dev, &clkdata->clkout_hw); - if (!clkdata->clkout) { - ret = -EINVAL; + if (IS_ERR(clkdata->clkout)) { + ret = PTR_ERR(clkdata->clkout); goto err_fll; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 2/3] clk: Add devm_clk_{register,unregister}() 2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd @ 2012-09-24 20:38 ` Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd 2012-10-29 18:13 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Mike Turquette 2 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2012-09-24 20:38 UTC (permalink / raw) To: linux-arm-kernel Some clock drivers can be simplified if devres takes care of unregistering any registered clocks along error paths. Introduce devm_clk_register() so that clock drivers get unregistration for free along with simplified error paths. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- v2: Use devres_release() drivers/clk/clk.c | 111 +++++++++++++++++++++++++++++++++++-------- include/linux/clk-provider.h | 2 + 2 files changed, 92 insertions(+), 21 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 56e4495e..6852809 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -17,6 +17,7 @@ #include <linux/list.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/device.h> static DEFINE_SPINLOCK(enable_lock); static DEFINE_MUTEX(prepare_lock); @@ -1361,28 +1362,9 @@ struct clk *__clk_register(struct device *dev, struct clk_hw *hw) } EXPORT_SYMBOL_GPL(__clk_register); -/** - * clk_register - allocate a new clock, register it and return an opaque cookie - * @dev: device that is registering this clock - * @hw: link to hardware-specific clock data - * - * clk_register is the primary interface for populating the clock tree with new - * clock nodes. It returns a pointer to the newly allocated struct clk which - * cannot be dereferenced by driver code but may be used in conjuction with the - * rest of the clock API. In the event of an error clk_register will return an - * error code; drivers must test for an error code after calling clk_register. - */ -struct clk *clk_register(struct device *dev, struct clk_hw *hw) +static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk) { int i, ret; - struct clk *clk; - - clk = kzalloc(sizeof(*clk), GFP_KERNEL); - if (!clk) { - pr_err("%s: could not allocate clk\n", __func__); - ret = -ENOMEM; - goto fail_out; - } clk->name = kstrdup(hw->init->name, GFP_KERNEL); if (!clk->name) { @@ -1420,7 +1402,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw) ret = __clk_init(dev, clk); if (!ret) - return clk; + return 0; fail_parent_names_copy: while (--i >= 0) @@ -1429,6 +1411,36 @@ fail_parent_names_copy: fail_parent_names: kfree(clk->name); fail_name: + return ret; +} + +/** + * clk_register - allocate a new clock, register it and return an opaque cookie + * @dev: device that is registering this clock + * @hw: link to hardware-specific clock data + * + * clk_register is the primary interface for populating the clock tree with new + * clock nodes. It returns a pointer to the newly allocated struct clk which + * cannot be dereferenced by driver code but may be used in conjuction with the + * rest of the clock API. In the event of an error clk_register will return an + * error code; drivers must test for an error code after calling clk_register. + */ +struct clk *clk_register(struct device *dev, struct clk_hw *hw) +{ + int ret; + struct clk *clk; + + clk = kzalloc(sizeof(*clk), GFP_KERNEL); + if (!clk) { + pr_err("%s: could not allocate clk\n", __func__); + ret = -ENOMEM; + goto fail_out; + } + + ret = _clk_register(dev, hw, clk); + if (!ret) + return clk; + kfree(clk); fail_out: return ERR_PTR(ret); @@ -1444,6 +1456,63 @@ EXPORT_SYMBOL_GPL(clk_register); void clk_unregister(struct clk *clk) {} EXPORT_SYMBOL_GPL(clk_unregister); +static void devm_clk_release(struct device *dev, void *res) +{ + clk_unregister(res); +} + +/** + * devm_clk_register - resource managed clk_register() + * @dev: device that is registering this clock + * @hw: link to hardware-specific clock data + * + * Managed clk_register(). Clocks returned from this function are + * automatically clk_unregister()ed on driver detach. See clk_register() for + * more information. + */ +struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw) +{ + struct clk *clk; + int ret; + + clk = devres_alloc(devm_clk_release, sizeof(*clk), GFP_KERNEL); + if (!clk) + return ERR_PTR(-ENOMEM); + + ret = _clk_register(dev, hw, clk); + if (!ret) { + devres_add(dev, clk); + } else { + devres_free(clk); + clk = ERR_PTR(ret); + } + + return clk; +} +EXPORT_SYMBOL_GPL(devm_clk_register); + +static int devm_clk_match(struct device *dev, void *res, void *data) +{ + struct clk *c = res; + if (WARN_ON(!c)) + return 0; + return c == data; +} + +/** + * devm_clk_unregister - resource managed clk_unregister() + * @clk: clock to unregister + * + * Deallocate a clock allocated with devm_clk_register(). Normally + * this function will not need to be called and the resource management + * code will ensure that the resource is freed. + */ +void devm_clk_unregister(struct device *dev, struct clk *clk) +{ + WARN_ON(devres_release(dev, devm_clk_release, devm_clk_match, clk)); +} +EXPORT_SYMBOL_GPL(devm_clk_unregister); + /*** clk rate change notifiers ***/ /** diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index c127315..710c6cb 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -327,8 +327,10 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, * error code; drivers must test for an error code after calling clk_register. */ struct clk *clk_register(struct device *dev, struct clk_hw *hw); +struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw); void clk_unregister(struct clk *clk); +void devm_clk_unregister(struct device *dev, struct clk *clk); /* helper functions */ const char *__clk_get_name(struct clk *clk); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 3/3] clk: wm831x: Use devm_clk_register() to simplify code 2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd @ 2012-09-24 20:38 ` Stephen Boyd 2012-10-29 18:13 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Mike Turquette 2 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2012-09-24 20:38 UTC (permalink / raw) To: linux-arm-kernel Move this driver to use devm_clk_register() to simplify some error paths and reduce lines of code. Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- v2: No changes drivers/clk/clk-wm831x.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c index eb1afaf..db4fbf2 100644 --- a/drivers/clk/clk-wm831x.c +++ b/drivers/clk/clk-wm831x.c @@ -370,43 +370,27 @@ static __devinit int wm831x_clk_probe(struct platform_device *pdev) clkdata->xtal_ena = ret & WM831X_XTAL_ENA; clkdata->xtal_hw.init = &wm831x_xtal_init; - clkdata->xtal = clk_register(&pdev->dev, &clkdata->xtal_hw); + clkdata->xtal = devm_clk_register(&pdev->dev, &clkdata->xtal_hw); if (IS_ERR(clkdata->xtal)) return PTR_ERR(clkdata->xtal); clkdata->fll_hw.init = &wm831x_fll_init; - clkdata->fll = clk_register(&pdev->dev, &clkdata->fll_hw); - if (IS_ERR(clkdata->fll)) { - ret = PTR_ERR(clkdata->fll); - goto err_xtal; - } + clkdata->fll = devm_clk_register(&pdev->dev, &clkdata->fll_hw); + if (IS_ERR(clkdata->fll)) + return PTR_ERR(clkdata->fll); clkdata->clkout_hw.init = &wm831x_clkout_init; - clkdata->clkout = clk_register(&pdev->dev, &clkdata->clkout_hw); - if (IS_ERR(clkdata->clkout)) { - ret = PTR_ERR(clkdata->clkout); - goto err_fll; - } + clkdata->clkout = devm_clk_register(&pdev->dev, &clkdata->clkout_hw); + if (IS_ERR(clkdata->clkout)) + return PTR_ERR(clkdata->clkout); dev_set_drvdata(&pdev->dev, clkdata); return 0; - -err_fll: - clk_unregister(clkdata->fll); -err_xtal: - clk_unregister(clkdata->xtal); - return ret; } static int __devexit wm831x_clk_remove(struct platform_device *pdev) { - struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); - - clk_unregister(clkdata->clkout); - clk_unregister(clkdata->fll); - clk_unregister(clkdata->xtal); - return 0; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking 2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd 2012-09-24 20:38 ` [PATCHv2 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd @ 2012-10-29 18:13 ` Mike Turquette 2 siblings, 0 replies; 16+ messages in thread From: Mike Turquette @ 2012-10-29 18:13 UTC (permalink / raw) To: linux-arm-kernel Quoting Stephen Boyd (2012-09-24 13:38:03) > clk_register() returns an ERR_PTR upon failure, not NULL. Fix > these error paths. > > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Thanks for sending V2. Taken into clk-next. Regards, Mike > --- > > v2: No changes > > drivers/clk/clk-wm831x.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c > index e7b7765..eb1afaf 100644 > --- a/drivers/clk/clk-wm831x.c > +++ b/drivers/clk/clk-wm831x.c > @@ -371,20 +371,20 @@ static __devinit int wm831x_clk_probe(struct platform_device *pdev) > > clkdata->xtal_hw.init = &wm831x_xtal_init; > clkdata->xtal = clk_register(&pdev->dev, &clkdata->xtal_hw); > - if (!clkdata->xtal) > - return -EINVAL; > + if (IS_ERR(clkdata->xtal)) > + return PTR_ERR(clkdata->xtal); > > clkdata->fll_hw.init = &wm831x_fll_init; > clkdata->fll = clk_register(&pdev->dev, &clkdata->fll_hw); > - if (!clkdata->fll) { > - ret = -EINVAL; > + if (IS_ERR(clkdata->fll)) { > + ret = PTR_ERR(clkdata->fll); > goto err_xtal; > } > > clkdata->clkout_hw.init = &wm831x_clkout_init; > clkdata->clkout = clk_register(&pdev->dev, &clkdata->clkout_hw); > - if (!clkdata->clkout) { > - ret = -EINVAL; > + if (IS_ERR(clkdata->clkout)) { > + ret = PTR_ERR(clkdata->clkout); > goto err_fll; > } > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-10-29 18:13 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19 6:05 [PATCH 0/3] Introduce devm_clk_register() Stephen Boyd
2012-09-19 6:05 ` [PATCH 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd
2012-09-20 1:31 ` Mark Brown
2012-09-19 6:05 ` [PATCH 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd
2012-09-21 2:33 ` Stephen Boyd
2012-09-22 1:07 ` Mike Turquette
2012-09-22 7:20 ` Stephen Boyd
2012-09-19 6:05 ` [PATCH 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd
2012-09-20 1:40 ` Mark Brown
2012-09-22 10:06 ` [PATCH 0/3] Introduce devm_clk_register() Russell King - ARM Linux
2012-09-24 20:20 ` Stephen Boyd
2012-09-24 20:35 ` Russell King - ARM Linux
2012-09-24 20:38 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Stephen Boyd
2012-09-24 20:38 ` [PATCHv2 2/3] clk: Add devm_clk_{register,unregister}() Stephen Boyd
2012-09-24 20:38 ` [PATCHv2 3/3] clk: wm831x: Use devm_clk_register() to simplify code Stephen Boyd
2012-10-29 18:13 ` [PATCHv2 1/3] clk: wm831x: Fix clk_register() error code checking Mike Turquette
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).