Re: [PATCH 2/7] OMAP: omap_device: Create clkdev entry for hwmod main_clk

[Todd Poynor <toddpoynor@google.com>]

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