From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752589AbcGLWVR (ORCPT <rfc822;w@1wt.eu>);
	Tue, 12 Jul 2016 18:21:17 -0400
Received: from pandora.armlinux.org.uk ([78.32.30.218]:52190 "EHLO
	pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752105AbcGLWVP (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 12 Jul 2016 18:21:15 -0400
Date: Tue, 12 Jul 2016 23:20:29 +0100
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Moritz Fischer <moritz.fischer@ettus.com>
Cc: Alan Tull <atull@opensource.altera.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Devicetree List <devicetree@vger.kernel.org>,
        Ian Campbell <ijc+devicetree@hellion.org.uk>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Dinh Nguyen <dinguyen@opensource.altera.com>,
        Alan Tull <delicious.quinoa@gmail.com>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2] fpga-manager: Add Socfpga Arria10 support
Message-ID: <20160712222029.GW1041@n2100.armlinux.org.uk>
References: <20160712190709.5964-1-atull@opensource.altera.com>
 <20160712190709.5964-3-atull@opensource.altera.com>
 <CAAtXAHfSBzLisqBgF4Vry0vrcL3+DB4b1ctMOrTdOq5ue4D7JQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAAtXAHfSBzLisqBgF4Vry0vrcL3+DB4b1ctMOrTdOq5ue4D7JQ@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jul 12, 2016 at 02:31:05PM -0700, Moritz Fischer wrote:
> On Tue, Jul 12, 2016 at 12:07 PM, Alan Tull <atull@opensource.altera.com> wrote:
> > +       priv->clk = devm_clk_get(dev, NULL);
> > +       if (IS_ERR(priv->clk)) {
> > +               dev_err(dev, "no clock specified\n");
> > +               return PTR_ERR(priv->clk);
> > +       }
> > +
> > +       ret = clk_prepare_enable(priv->clk);
> > +       if (ret) {
> > +               dev_err(dev, "could not enable clock\n");
> > +               clk_put(priv->clk);
> 
> Seen that you used devm_clk_get() is this one necessary?

That's actually a bug.  Never clk_put() a devm_clk_get()'d clock.
devm_clk_put() is what you want if provided.  However, I think
this clk_put() call is useless here.

> > +static int socfpga_a10_fpga_remove(struct platform_device *pdev)
> > +{
> > +       struct fpga_manager *mgr = platform_get_drvdata(pdev);
> > +       struct a10_fpga_priv *priv = mgr->priv;
> > +
> > +       fpga_mgr_unregister(&pdev->dev);
> > +       clk_disable_unprepare(priv->clk);
> > +       clk_put(priv->clk);
> 
> Same here, if needed at all shouldn't it be devm_clk_put() ?

And also useless here, as the whole point of the devm_* stuff is to
clean up the resources that were claimed on probe failure or when
the device is unbound from its driver.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.