From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-bk0-f48.google.com ([209.85.214.48]:55567 "EHLO mail-bk0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754821Ab3GZNg0 (ORCPT ); Fri, 26 Jul 2013 09:36:26 -0400 Received: by mail-bk0-f48.google.com with SMTP id jf3so1134321bkc.7 for ; Fri, 26 Jul 2013 06:36:25 -0700 (PDT) Message-ID: <51F27B56.9050504@gmail.com> Date: Fri, 26 Jul 2013 15:36:22 +0200 From: Sylwester Nawrocki MIME-Version: 1.0 To: Laurent Pinchart CC: linux-media@vger.kernel.org, Mike Turquette Subject: Re: [PATCH] mt9v032: Use the common clock framework References: <1373021725-14006-1-git-send-email-laurent.pinchart@ideasonboard.com> <51F2756C.70507@gmail.com> <1438897.qqa6gsnOmc@avalon> In-Reply-To: <1438897.qqa6gsnOmc@avalon> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, On 07/26/2013 03:15 PM, Laurent Pinchart wrote: > On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: >> On 07/05/2013 12:55 PM, Laurent Pinchart wrote: >>> Configure the device external clock using the common clock framework >>> instead of a board code callback function. >>> >>> Signed-off-by: Laurent Pinchart >>> --- >>> >>> drivers/media/i2c/mt9v032.c | 16 ++++++++++------ >>> include/media/mt9v032.h | 4 ---- >>> 2 files changed, 10 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c >>> index 60c6f67..7b30640 100644 >>> --- a/drivers/media/i2c/mt9v032.c >>> +++ b/drivers/media/i2c/mt9v032.c >>> @@ -12,6 +12,7 @@ >>> * published by the Free Software Foundation. >>> */ >>> >>> +#include >>> #include >>> #include >>> #include >>> @@ -135,6 +136,8 @@ struct mt9v032 { >>> struct mutex power_lock; >>> int power_count; >>> >>> + struct clk *clk; >>> + >>> struct mt9v032_platform_data *pdata; >>> >>> u32 sysclk; >>> @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) >>> struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); >>> int ret; >>> >>> - if (mt9v032->pdata->set_clock) { >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); >>> - udelay(1); >>> - } >>> + clk_prepare_enable(mt9v032->clk); >>> + udelay(1); >>> >>> /* Reset the chip and stop data read out */ >>> ret = mt9v032_write(client, MT9V032_RESET, 1); >>> @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) >>> >>> static void mt9v032_power_off(struct mt9v032 *mt9v032) >>> { >>> - if (mt9v032->pdata->set_clock) >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); >>> + clk_disable_unprepare(mt9v032->clk); >>> } >>> >>> static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) >>> @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, >>> if (!mt9v032) >>> return -ENOMEM; >>> >>> + mt9v032->clk = devm_clk_get(&client->dev, NULL); >>> + if (IS_ERR(mt9v032->clk)) >>> + return PTR_ERR(mt9v032->clk); >>> + >>> mutex_init(&mt9v032->power_lock); >>> mt9v032->pdata = pdata; >>> >>> diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h >>> index 78fd39e..12175a6 100644 >>> --- a/include/media/mt9v032.h >>> +++ b/include/media/mt9v032.h >>> @@ -1,13 +1,9 @@ >>> #ifndef _MEDIA_MT9V032_H >>> #define _MEDIA_MT9V032_H >>> >>> -struct v4l2_subdev; >>> - >>> struct mt9v032_platform_data { >>> unsigned int clk_pol:1; >>> - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); >>> - >>> const s64 *link_freqs; >>> s64 link_def_freq; >>> }; >> >> Is there clk_put() somewhere in this patch ? I would expect it somewhere >> around driver remove() callback, but can't see it. :-/ > > There's *devm_*clk_get() instead :-) Ah, I knew I must have been forgetting or overlooking something! ;) Do you rely on the fact that __clk_get()/__clk_put() doesn't get reference on the clock supplier module (to avoid locking modules in memory) ? I was planning on adding module_get()/module_put() inside __clk_get()/__clk_out() for the common clock API implementation. Regards, Sylwester