linux-kernel - Re: [PATCH] regulator: stub out devm_regulator_get

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: <20141024211848.GP3729@sirena.org.uk>
Date:	Fri, 24 Oct 2014 22:18:48 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Rob Clark <robdclark@...il.com>
Cc:	Felipe Balbi <balbi@...com>, lgirdwood@...il.com,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	David Airlie <airlied@...ux.ie>,
	David Brown <davidb@...eaurora.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] regulator: stub out devm_regulator_get_exclusive

On Fri, Oct 24, 2014 at 04:36:24PM -0400, Rob Clark wrote:

> iirc, I was using _get_exclusive() in a few places where I wanted to
> be sure not to get dummy-regulator in cases where I should
> -EPROBE_DEFER instead (since probe order with DT is slightly
> hilarious, and since I depend on a few other drivers I end up
> deferring at least a couple times at boot)... I don't quite remember
> the details.  But afaict regulator_get() still allows dummy-regulator,
> which is what I specifically don't want.

No, this is actually making things worse.  You will only get a dummy
regulator from regulator_get() if no regulator at all is mapped in the
DT, if one is mapped then you'll always get either an -EPROBE_DEFER or
the real regulator.  Right now the driver is broken with respect to
-EPROBE_DEFER since it just completely ignores the error code and
carries on happily if any error is returned which means that the
behaviour is going to be unstable on any given system, what happens will
depend on probe order which could in turn depend on what's been built
modular and so on.

As far as I can tell the only thing the driver does with the regulator
it's grabbing exclusively is enable it in probe() and that's going to
work just as well with the dummy regulator anyway so I can't see any
reason to worry if the driver is getting one.

> If you have a recommendation for a better way, I am all ears.

Just use regulator_get() (or better, devm_regulator_get()) and pay
attention to the errors.  The driver should also disable the regulator
on remove and I'd be surprised if the other two regulators shouldn't be
using a normal _get() too.  If there is a good reason to use _optional()
then the code should be changed to use ERR_PTR() rather than NULL to
check for missing regulators and the driver needs to keep them enabled
as long as it's using them.

Given that the two optional regulators are only set to one specific
value it's a bit surprising that the DT doesn't do this but I guess it's
possible there could be broken DTs out there that do give permission for
set_voltage() for a range rather than specifying the correct voltage.

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