linux-kernel - Re: [PATCH v4 4/4] ARM: mvebu: Armada 385 GP: Add regulators to the SATA port

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: <20150116202516.GL3856@sirena.org.uk>
Date:	Fri, 16 Jan 2015 20:25:16 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Hans de Goede <hdegoede@...hat.com>
Cc:	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Tejun Heo <tj@...nel.org>, linux-ide@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Boris BREZILLON <boris.brezillon@...e-electrons.com>,
	Jason Cooper <jason@...edaemon.net>,
	Andrew Lunn <andrew@...n.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	linux-arm-kernel@...ts.infradead.org,
	Lior Amsalem <alior@...vell.com>,
	Tawfik Bayouk <tawfik@...vell.com>,
	Nadav Haklai <nadavh@...vell.com>,
	Mark Rutland <mark.rutland@....com>, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 4/4] ARM: mvebu: Armada 385 GP: Add regulators to the
 SATA port

On Fri, Jan 16, 2015 at 08:12:28PM +0100, Hans de Goede wrote:
> On 16-01-15 13:37, Mark Brown wrote:

> >>It is probably a good idea to use regulator-boot-on and
> >>then test things this way, and if that works use
> >>regulator-boot-on.

> >No, it's unlikely that boot-on makes sense here - it's there for cases
> >where we can't read back the hardware state at power on.

> Which we cannot here since we've a gpio driven regulator, with the pin
> in output mode, so we cannot read it back. Or at least the current kernel
> implementation relies on regulator-boot-on rather then reading the
> state back from the hardware.

Right, this always seems like a problem with the GPIO API - not being
able to read the state back is a total pain for boot handover, we should
really have a way of reading back the current output state.  This is one
of the few cases where boot-on can be useful, though ideally it'd be set
by the bootloader rather than as a static thing since otherwise you have
the opposite problem in the case where the disk isn't being used at the
current time.

> While we're discussing this I would also like to advocate to change the
> behavior for regulator-boot-on to be the same as always-on when it comes
> to regulator_init_complete(), iow regulator-boot-on regulators should
> not be touched by regulator_init_complete(). It makes no sense to have
> different behavior for build in versus module drivers here.

Like I say boot-on is *only* intended to support bootstrapping, if you
want the regulator to be always on then mark it as always on.  If you
want some way of marking the end of userspace init so we can defer
cleanup of unused resources then go do that - it's a change I've
advocated before, it'd help with modular drivers in general.

> I believe we should still leave these on once userspace starts running,
> there is a reason they are marked as regulator-boot-on and the fact that
> we're ready to start running userspace does not take away that reason, to
> me regulator-boot-on means "do not turn off until explicitly told so by
> a driver which (hopefully) knows wtf it is doing".

No, that's not it at all - it just means that either the hardware didn't
support readback or we're working around our own limitations in not
being capable of doing that.  Remember, regulators are reference counted
so with a few exceptions drivers aren't saying "turn this off now"
they're saying "this device doesn't need this regulator right now, turn
it off if you like" and there's no guarantee that a driver will ever be
loaded in the first place.

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