From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754453AbbCBSrq (ORCPT ); Mon, 2 Mar 2015 13:47:46 -0500 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:32885 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753862AbbCBSro (ORCPT ); Mon, 2 Mar 2015 13:47:44 -0500 Date: Mon, 2 Mar 2015 18:47:22 +0000 From: Mark Brown To: Doug Anderson Cc: milo.kim@ti.com, axel.lin@ingics.com, Dmitry Torokhov , olof@lixom.net, javier.martinez@collabora.co.uk, Paul Stewart , stable@vger.kernel.org, lgirdwood@gmail.com, linux-kernel@vger.kernel.org Message-ID: <20150302184722.GE21293@sirena.org.uk> References: <1425066064-18920-1-git-send-email-dianders@chromium.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="VefyKIJnaeAk0cwy" Content-Disposition: inline In-Reply-To: <1425066064-18920-1-git-send-email-dianders@chromium.org> X-Cookie: My LESLIE GORE record is BROKEN ... User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 94.175.94.161 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH] regulator: core: Fix enable GPIO reference counting X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --VefyKIJnaeAk0cwy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 27, 2015 at 11:41:03AM -0800, Doug Anderson wrote: > It is possible for _regulator_do_enable() to be called for an > already-enabled rdev, like in regulator_suspend_finish(). If we were > using an enable pin (rdev->ena_pin is set) then we'd end up > incrementing the reference count in regulator_ena_gpio_ctrl() over and > over again without a decrement. That prevented the GPIO from going to > the "off" state even after all users were disabled. > Fix this by avoiding the call to regulator_ena_gpio_ctrl() when it's > not needed. There's a big jump in this changelog where you assert that we're avoiding the call "when it's not needed" without explaining the situations in which this is the case or why. =20 Looking at the code it seems that you're adding checks to skip calls in the standard enable and disable paths but not touching other paths, based on this patch by itself I can't tell if this is a good idea or not. It certainly doesn't feel robust - if we're missing reference counting skipping operations seems likely to lead to other bugs popping up elsewhere when the other user that isn't doing a disable currently decides to start doing so. > Signed-off-by: Doug Anderson > Fixes: 967cfb18c0e3 ("regulator: core: manage enable GPIO list") Fixes normally goes first. --VefyKIJnaeAk0cwy Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJU9LA5AAoJECTWi3JdVIfQmjYH/RAcFGSEdONizpbEhkGZaM6Q yJ9FsJgjnWzU7di5jEDI/tN+JVpa1M9aD63pqLd4ZSNlKIBIKGacD6i4iS/UcE2a +y2uQaZz7M0XEEnbVoOZ0LLu3cCowz1IV307/k++LXicNuzmNXx8SS7O6W1y2+5H 1chRwLTnWQ7GpwHT3OlP/lQhI3cJk9vzD/IiLP3glUzJA5TkkvpnfeMHxlR9hTi+ n3DoBb5rmB5WIgbpikk5FUM7YF/sMAcqS0X24Qech70QovbBhNGv04Yl4i75pNHB 3Od/y23+8/9jdl7BaxpLBB+lqpM1Ij8lOW2rTuMGJ9EQqYBm49sd1DZaf038TjY= =FCaI -----END PGP SIGNATURE----- --VefyKIJnaeAk0cwy--