From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754450AbbCBS53 (ORCPT ); Mon, 2 Mar 2015 13:57:29 -0500 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:32907 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753590AbbCBS52 (ORCPT ); Mon, 2 Mar 2015 13:57:28 -0500 Date: Mon, 2 Mar 2015 18:57:13 +0000 From: Mark Brown To: Javier Martinez Canillas Cc: Doug Anderson , milo.kim@ti.com, axel.lin@ingics.com, Dmitry Torokhov , olof@lixom.net, Paul Stewart , stable@vger.kernel.org, lgirdwood@gmail.com, linux-kernel@vger.kernel.org Message-ID: <20150302185713.GF21293@sirena.org.uk> References: <1425066064-18920-1-git-send-email-dianders@chromium.org> <54F0DB23.3010203@collabora.co.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XpnDihzI2tYfJNuq" Content-Disposition: inline In-Reply-To: <54F0DB23.3010203@collabora.co.uk> 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 --XpnDihzI2tYfJNuq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 27, 2015 at 10:01:23PM +0100, Javier Martinez Canillas wrote: > I noticed the same problem in regulator_suspend_finish() when I was working > on S2R for Exynos a couple of months ago and had patch [0] on my local tree > but never found the time to do extensive testing so I never posted it. Please don't bury patches in the middle of mails where they're hard to apply if they're useful. > I see that the check is already in _regulator_enable() so another option > is to call _regulator_enable() instead of _regulator_do_enable() in > regulator_suspend_finish(). I'm not entirely sure what "the check" is? > Trying to enable an already enabled regulator may cause issues so is > better to skip enabling regulators that were not disabled before suspend. > mutex_lock(&rdev->mutex); > if (rdev->use_count > 0 || rdev->constraints->always_on) { > - error = _regulator_do_enable(rdev); > - if (error) > - ret = error; > + if (!_regulator_is_enabled(rdev)) { > + error = _regulator_do_enable(rdev); > + if (error) > + ret = error; > + } This seems like a better fix or at least a better approach - essentially the assumption in most of the code is that regulator enables are just register writes so repeated updates don't have any effect. We may need a specific per client count here... I've not looked at the code and I only got back to the UK this morning so I'm not going to start now. --XpnDihzI2tYfJNuq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJU9LKIAAoJECTWi3JdVIfQ/agH/joyrcMXID5iBGcdEsRf6LYq M3EVpL1nQhnRqNOsZ9HnyvQa62TDUn6Yzymm7A/pncBk/oENgj+f7h5f7keYKuVu IZZg3xGx0rbADVH2R/RMKc9TdHIWkt98TW+mAjeUHNcvFObfu+FtKn+k45WuLt3c hiuAxgqp1IJyuFWIpg9E6vNqUFSMprYqzU0Zy9nWRVcqsWt/9u/58SDge6PZCAsM hfwHpPebtXNMD9KuaZwPtO5EYeHLF6c1QeiSwat8+0N5UgCMpxGAMhvUhERYhBYh Wu5itnphLALCYhqaUXGFxTie5TLIsrqXBiWk/FvPP0ubily51t6zTghXej8koRI= =2+8A -----END PGP SIGNATURE----- --XpnDihzI2tYfJNuq--