From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966654AbbBDQEd (ORCPT ); Wed, 4 Feb 2015 11:04:33 -0500 Received: from mga02.intel.com ([134.134.136.20]:40796 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965247AbbBDQEb (ORCPT ); Wed, 4 Feb 2015 11:04:31 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,518,1418112000"; d="scan'208";a="672697252" Message-ID: <1423065835.31903.424.camel@linux.intel.com> Subject: Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev() From: Andy Shevchenko To: Mika Westerberg Cc: linux-kernel@vger.kernel.org, Stephen Boyd , Mike Turquette , Lee Jones , Bryan Huntsman , Lorenzo Pieralisi , Ralf Baechle , Sylwester Nawrocki , Tomeu Vizoso Date: Wed, 04 Feb 2015 18:03:55 +0200 In-Reply-To: <20150204155054.GK18758@lahna.fi.intel.com> References: <1422987534-21832-1-git-send-email-andriy.shevchenko@linux.intel.com> <1422987534-21832-2-git-send-email-andriy.shevchenko@linux.intel.com> <20150204155054.GK18758@lahna.fi.intel.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-02-04 at 17:50 +0200, Mika Westerberg wrote: > On Tue, Feb 03, 2015 at 08:18:54PM +0200, Andy Shevchenko wrote: > > Since clk_register_clkdev() is exported for modules the caller should get a > > pointer to the allocated resources. Otherwise the memory leak is guaranteed on > > the ->remove() stage. > > > > Cc: Tomeu Vizoso > > Signed-off-by: Andy Shevchenko > > One comment, see below. Nothing major so feel free to add, > > Reviewed-by: Mika Westerberg Thanks for review! Though still wait for others to comment and test if possible. By the way, I have to add that we have one unpublished (yet) user of this. So, we are testing this internally on x86. Just to note that the user is a module which might be unloaded. That's why important to release the acquired resources. > > > --- > > arch/arm/mach-msm/clock-pcom.c | 9 +++++---- > > arch/arm/mach-vexpress/spc.c | 5 ++++- > > arch/mips/ath79/clock.c | 6 +++--- > > drivers/clk/clk-bcm2835.c | 12 +++++++----- > > drivers/clk/clk-max-gen.c | 9 ++++----- > > drivers/clk/clk-xgene.c | 6 +++--- > > drivers/clk/clkdev.c | 14 +++++++++----- > > drivers/clk/samsung/clk-pll.c | 13 ++++++++----- > > drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++--------- > > drivers/clk/samsung/clk.c | 35 +++++++++++++++++++--------------- > > include/linux/clkdev.h | 2 +- > > 11 files changed, 74 insertions(+), 56 deletions(-) > > ... > > > index 6e5c504..e07b1e2 100644 > > --- a/drivers/clk/clkdev.c > > +++ b/drivers/clk/clkdev.c > > @@ -307,29 +307,33 @@ EXPORT_SYMBOL(clkdev_drop); > > * clkdev. > > * > > * To make things easier for mass registration, we detect error clks > > - * from a previous clk_register() call, and return the error code for > > + * from a previous clk_register() call, and return the error pointer for > > * those. This is to permit this function to be called immediately > > * after clk_register(). > > + * > > + * Return: > > + * pointer to the allocated struct clk_lookup on success, or error pointer > > + * otherwise. > > This should probably say how these resources are supposed to be > released. Agree. I would add this in v2. -- Andy Shevchenko Intel Finland Oy