From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751607AbdK2S3u (ORCPT ); Wed, 29 Nov 2017 13:29:50 -0500 Received: from heliosphere.sirena.org.uk ([172.104.155.198]:37492 "EHLO heliosphere.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750713AbdK2S3s (ORCPT ); Wed, 29 Nov 2017 13:29:48 -0500 Date: Wed, 29 Nov 2017 18:29:45 +0000 From: Mark Brown To: Maciej Purski Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Liam Girdwood , Rob Herring , Mark Rutland , Marek Szyprowski , Bartlomiej Zolnierkiewicz Subject: Re: [RFC PATCH v2 3/3] regulator: core: Balance coupled regulators voltages Message-ID: <20171129182945.t52kuz6ezogsdvej@sirena.org.uk> References: <1508330822-8039-1-git-send-email-m.purski@samsung.com> <1508330822-8039-4-git-send-email-m.purski@samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="wwi4xzhjx66hxlqn" Content-Disposition: inline In-Reply-To: <1508330822-8039-4-git-send-email-m.purski@samsung.com> X-Cookie: Pushing 40 is exercise enough. User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --wwi4xzhjx66hxlqn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Oct 18, 2017 at 02:47:02PM +0200, Maciej Purski wrote: > +static void regulator_lock_supply_parents(struct regulator_dev *rdev) > +{ > + struct regulator_dev *supply = rdev_get_supply(rdev); > + > + if (supply) > + regulator_lock_supply(supply); > +} These functions are fairly pointless as they're so small, and they're misnamed as they only lock a single parent but the name suggests it's going to lock multiple things (I'd expect all the coupled regulators or something from the name). > + /* > + * If the regulator is coupled with other regulators, we have to > + * balance their voltages to keep the max_spread constraint. > + */ > + if (rdev->coupled_desc) > + regulator_balance_coupled(rdev->coupled_desc); Just put the check into regulator_balance_coupled() rather than doing it at every call site. > + /* > + * If the regulator is coupled, return after changing consumer demands > + * without changing voltage. This will be handled outside the function > + * by regulator_balance_coupled() > + */ > + if (rdev->coupled_desc) > + goto out; > + > + ret = regulator_set_voltage_rdev(regulator->rdev, min_uV, max_uV); > + if (ret < 0) > + goto out2; Where is the elsewhere? I'm worried this is going to make things more confusing. --wwi4xzhjx66hxlqn Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAloe/JgACgkQJNaLcl1U h9AGEwf9GPDs7KNX0q4OcbIaztgJk/snxn/8BvCxFYGFNL1ymwr6U/BZf96n2WVp HYYwP2akvO44tHGGp+g5CBXUrwQUPiqf68aFs0TBEVOe8YqvbA8Fhp3UpQe43fKx X6zZo6kFHastQl9cDGlhCDsrDfjgNOoX3sSuQbtMhcf53h6+x91DnItvbt97ORwa VPbJX2b9jW7acoDmTaUksnvde9bmx3/g/Tk/uEK8EVsT41pO7sCwkrgMokw+XTUO Va3Ifma5bC4UyiZz5ew/9f/YDtuLti0hUDwsfOUtJawPcPzvvKxII9qPuJQDMZ0J qyrQZ7CkhwmQpgJJTBYW98hhfuWC2A== =jKun -----END PGP SIGNATURE----- --wwi4xzhjx66hxlqn--