linux-kernel - Re: [PATCH 1/1] regulator: Only enable disabled regulators on resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150308193812.GW28806@sirena.org.uk>
Date:	Sun, 8 Mar 2015 19:38:12 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
Cc:	Doug Anderson <dianders@...omium.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] regulator: Only enable disabled regulators on resume

On Wed, Mar 04, 2015 at 02:45:00PM +0100, Javier Martinez Canillas wrote:

> The thing is that _regulator_is_enabled() used to return -EINVAL if
> the rdev didn't have an .is_enabled callback but that changed in
> commit 9a7f6a4c6edc8 ("regulator: Assume regulators are enabled if
> they don't report anything") and now returns 1 in that case. But
> _regulator_enable() was not changed and is still checking for -EINVAL
> which seems to me like a left over after the mentioned commit.

You mean _do_enable(), not _enable() here.  It's not really a leftover
as the two operations are doing somewhat different things and the
changes are a bit separate, _is_enabled() is reporting the current state
while _do_enable() is making a change so it should fail if it can't do
that.  

A better way of writing it in the _do_enable() case is that it possibly
ought to be checking if the regulator is enabled before it does
anything, though for uncached regulator operations that then means an
extra I/O which isn't great.  Given that I think rather than ignoring
the missing op it should instead fall back to checking _is_enabled() -
that way if we can read the state but not change it the right thing will
happen.  I'll do a patch, probably tomorrow.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)