Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755297AbdCGMfO (ORCPT ); Tue, 7 Mar 2017 07:35:14 -0500 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:53390 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754997AbdCGMex (ORCPT ); Tue, 7 Mar 2017 07:34:53 -0500 Date: Tue, 7 Mar 2017 13:34:12 +0100 From: Mark Brown To: Venkat Reddy Talla Cc: lgirdwood@gmail.com, linux-kernel@vger.kernel.org, ldewangan@nvidia.com Message-ID: <20170307123412.yzij7oolqjnyzzr3@sirena.org.uk> References: <1488818218-25843-1-git-send-email-vreddytalla@nvidia.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qjbzre4lorautskh" Content-Disposition: inline In-Reply-To: <1488818218-25843-1-git-send-email-vreddytalla@nvidia.com> X-Cookie: A fool and his money are soon popular. User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 109.74.48.129 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH 1/2] regulator: tps65132: add regulator driver for TI TPS65132 X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: No (on mezzanine.sirena.org.uk); Unknown failure Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2274 Lines: 70 --qjbzre4lorautskh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Mar 06, 2017 at 10:06:58PM +0530, Venkat Reddy Talla wrote: > + dis_mask = (id == TPS65132_REGULATOR_ID_VPOS) ? > + TPS65132_REG_APPS_DISP : TPS65132_REG_APPS_DISN; Please don't abuse the ternery operator, write normal if statements so that people can read the code easily. > +static int tps65132_disable(struct regulator_dev *rdev) > +{ > + struct tps65132_regulator *tps = rdev_get_drvdata(rdev); > + int id = rdev_get_id(rdev); > + int act_dis_gpio = tps->reg_pdata[id].active_discharge_gpio; > + unsigned int act_dis_time_us = tps->reg_pdata[id].active_discharge_time; > + > + if (gpio_is_valid(act_dis_gpio)) { > + gpio_set_value_cansleep(act_dis_gpio, 1); > + usleep_range(act_dis_time_us, > + act_dis_time_us + TPS65132_ACT_DIS_TIME_SLACK); > + gpio_set_value_cansleep(act_dis_gpio, 0); > + } Don't we need to undo the active discharge register write? > +static int tps65132_get_regulator_dt_data(struct device *dev, > + struct tps65132_regulator *tps65132_regs) > +{ > + struct tps65132_reg_pdata *rpdata; > + struct device_node *rnode; > + int id; > + int ret; > + > + ret = of_regulator_match(dev, dev->of_node, tps65132_regulator_matches, > + ARRAY_SIZE(tps65132_regulator_matches)); > + if (ret < 0) { > + dev_err(dev, "node parsing for regulator failed: %d\n", ret); > + return ret; > + } Don't open code this, use of_parse_cb() to parse your extra properties. > +static int __init tps65132_init(void) > +{ > + return i2c_add_driver(&tps65132_i2c_driver); > +} > +subsys_initcall(tps65132_init); You should use module_i2c_driver() for new drivers. --qjbzre4lorautskh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAli+qMMACgkQJNaLcl1U h9DX9wf+NvoCc6ttSv1diJUNGT/EG9lR+08+t1utD571au6Lvc4hL5JR5a5c8Gxm GdOh7Bg3ZwAsmg/C3lt9TN314Am4U9H5RzRPajisFrIVLzHSFyvQ8FYRmvV7QgwS aPikFZFQS6Fm7wImwhTBeY/cOqA1VRASvj734bqkqeQ3rL4dLB8OEfzziDhaVX4o f0wiqfWCG3TknJAr1MsBCD1wex/65FNhQcfzl275XbCc/F4/hAgbIGwvlFlCFaDd 78av/xTrcV4oKmHx9gAZ5iQu8SdAj8ycQb4w0CXDrO7kNFUqTIe/cDK1DHQ4WzQ+ Bv9He1/8WwvfSk7NNOL0x/dFft6vzA== =urwf -----END PGP SIGNATURE----- --qjbzre4lorautskh--