On Tue, Jun 28, 2011 at 04:10:55PM +0200, Cousson, Benoit wrote: > On 6/27/2011 8:56 PM, Todd Poynor wrote: > >On Mon, Jun 27, 2011 at 06:33:06PM +0200, Benoit Cousson wrote: > >... > >>+ r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias); > >>+ if (!IS_ERR(r)) { > >>+ pr_warning("omap_device: %s: %s already exist\n", > >>+ dev_name(&od->pdev.dev), clk_alias); > > > >I believe a clk_put(r) is appropriate here. > > Appropriate I don't know, but useless for sure :-) > This clk_put is a no-op for every ARM platforms (I found one exception). I haven't followed the design of common struct clk closely, but it probably will do ref counting on these, so it's best to keep these matched up. > >>+ r = omap_clk_get_by_name(clk_name); > >>+ if (IS_ERR(r)) { > >>+ pr_err("omap_device: %s: omap_clk_get_by_name for %s failed\n", > >>+ dev_name(&od->pdev.dev), clk_name); > >>+ return; > >>+ } > >>+ > >>+ l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev)); > >>+ if (!l) { > >>+ pr_err("omap_device: %s: clkdev_alloc for %s failed\n", > >>+ dev_name(&od->pdev.dev), clk_alias); > > > >And here. > > No, it is not needed in that case because the omap_clk_get_by_name > is not using the clk_get API. Ah, didn't check that one, sorry. That's an unfortunate use of "get" in the API name IMO. When common struct clk takes over, this may need some tweaking. Todd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html