From patchwork Tue May 24 20:20:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Machek X-Patchwork-Id: 34419 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b5IpW-0006bC-Vb; Tue, 24 May 2016 20:21:30 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-5) with esmtp id 1b5IpU-0000zZ-8d; Tue, 24 May 2016 22:21:30 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754765AbcEXUVI (ORCPT + 1 other); Tue, 24 May 2016 16:21:08 -0400 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:55707 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751670AbcEXUVG (ORCPT ); Tue, 24 May 2016 16:21:06 -0400 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id D5A078240D; Tue, 24 May 2016 22:20:59 +0200 (CEST) Date: Tue, 24 May 2016 22:20:59 +0200 From: Pavel Machek To: Ivaylo Dimitrov Cc: pali.rohar@gmail.com, sre@kernel.org, kernel list , linux-arm-kernel , linux-omap@vger.kernel.org, tony@atomide.com, khilman@kernel.org, aaro.koskinen@iki.fi, patrikbachan@gmail.com, serge@hallyn.com, linux-media@vger.kernel.org, mchehab@osg.samsung.com, sakari.ailus@iki.fi Subject: Re: [PATCHv3] support for AD5820 camera auto-focus coil Message-ID: <20160524202059.GB18536@amd> References: <20160517181927.GA28741@amd> <20160521054336.GA27123@amd> <573FFF51.1000004@gmail.com> <20160521105607.GA20071@amd> <574049EF.2090208@gmail.com> <20160524090433.GA1277@amd> <57441BF8.60606@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <57441BF8.60606@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.5.24.201216 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, KNOWN_FREEWEB_URI 0.05, MSGID_ADDED_BY_MTA 0.05, BODY_SIZE_3000_3999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, INVALID_MSGID_NO_FQDN 0, IN_REP_TO 0, LEGITIMATE_NEGATE 0, MSG_THREAD 0, MULTIPLE_RCPTS_RND 0, NO_URI_HTTPS 0, REFERENCES 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CD 0, __CP_URI_IN_BODY 0, __CT 0, __CT_TEXT_PLAIN 0, __FORWARDED_MSG 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __KNOWN_FREEWEB_URI2 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __REFERENCES 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __URI_IN_BODY 0, __URI_NS , __URI_WITH_PATH 0, __USER_AGENT 0' Hi! > >>devm_regulator_get()? > > > >I'd rather avoid devm_ here. Driver is simple enough to allow it. > > > > Now thinking about it, what would happen here if regulator_get() returns > -EPROBE_DEFER? Wouldn't it be better to move regulator_get to the probe() > function, something like: Ok, I can do it. Oh, and don't try to complain about newlines before returns. It looks better this way. > static int ad5820_probe(struct i2c_client *client, > const struct i2c_device_id *devid) > { > struct ad5820_device *coil; > int ret = 0; > > coil = devm_kzalloc(sizeof(*coil), GFP_KERNEL); > if (coil == NULL) > return -ENOMEM; > > coil->vana = devm_regulator_get(&client->dev, NULL); > if (IS_ERR(coil->vana)) { > ret = PTR_ERR(coil->vana); > if (ret != -EPROBE_DEFER) > dev_err(&client->dev, "could not get regulator for vana\n"); > return ret; > } > > mutex_init(&coil->power_lock); > ... > > with the appropriate changes to remove() because of the devm API > usage. Something like this? diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c index f956bd3..f871366 100644 --- a/drivers/media/i2c/ad5820.c +++ b/drivers/media/i2c/ad5820.c @@ -8,7 +8,7 @@ * Copyright (C) 2016 Pavel Machek * * Contact: Tuukka Toivonen - * Sakari Ailus + * Sakari Ailus * * Based on af_d88.c by Texas Instruments. * @@ -263,13 +263,6 @@ static int ad5820_init_controls(struct ad5820_device *coil) static int ad5820_registered(struct v4l2_subdev *subdev) { struct ad5820_device *coil = to_ad5820_device(subdev); - struct i2c_client *client = v4l2_get_subdevdata(subdev); - - coil->vana = regulator_get(&client->dev, "VANA"); - if (IS_ERR(coil->vana)) { - dev_err(&client->dev, "could not get regulator for vana\n"); - return -ENODEV; - } return ad5820_init_controls(coil); } @@ -367,10 +360,18 @@ static int ad5820_probe(struct i2c_client *client, struct ad5820_device *coil; int ret = 0; - coil = kzalloc(sizeof(*coil), GFP_KERNEL); + coil = devm_kzalloc(sizeof(*coil), GFP_KERNEL); if (!coil) return -ENOMEM; + coil->vana = devm_regulator_get(&client->dev, NULL); + if (IS_ERR(coil->vana)) { + ret = PTR_ERR(coil->vana); + if (ret != -EPROBE_DEFER) + dev_err(&client->dev, "could not get regulator for vana\n"); + return ret; + } + mutex_init(&coil->power_lock); v4l2_i2c_subdev_init(&coil->subdev, client, &ad5820_ops); @@ -390,10 +391,6 @@ static int ad5820_probe(struct i2c_client *client, cleanup: media_entity_cleanup(&coil->subdev.entity); - -free: - kfree(coil); - return ret; } @@ -405,11 +402,6 @@ static int __exit ad5820_remove(struct i2c_client *client) v4l2_device_unregister_subdev(&coil->subdev); v4l2_ctrl_handler_free(&coil->ctrls); media_entity_cleanup(&coil->subdev.entity); - if (coil->vana) - regulator_put(coil->vana); - - kfree(coil); - return 0; }