linux-kernel - Re: [PATCH 5/5] ARM: dts: Add initial regulator mode on exynos Peach boards

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: <20141009174848.GW4609@sirena.org.uk>
Date:	Thu, 9 Oct 2014 18:48:48 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
Cc:	Doug Anderson <dianders@...omium.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Olof Johansson <olof@...om.net>,
	Chris Zhong <zyw@...k-chips.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Abhilash Kesavan <kesavan.abhilash@...il.com>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH 5/5] ARM: dts: Add initial regulator mode on exynos Peach
 boards

On Thu, Oct 09, 2014 at 04:27:37PM +0200, Javier Martinez Canillas wrote:

> I see, I thought that an operating mode could be anything that alter the
> regulator behavior either during runtime or when the system is suspended.
> But under your definition, it is true that most max77802 regulators have

It's not just me, it's the code and all the users and documentation...

> only two modes: ON and OFF (and some of them have a third Low Power mode).

...but let's be clear, only "on" (normal) and low power are modes here.
Like I keep saying please think things through - if modes also include
enable control why would they be treated separately in the API?

> I think though that a generic way to configure this enable control feature
> is needed. Maybe adding a new pair of .{get,set}_suspend_control function
> pointers to struct regulator_ops and an .initial_suspend_ctrl field to the
> struct regulation_constraints?

> That way the core could parse a generic DT property and call the function
> handlers but each driver can document in their own DT bindings what their
> control values are and how those affects the regulators during suspend?

That maps poorly onto a lot of devices which have control schemes which
are more complex than this, for example placing regulators into groups
which are then controlled en masse or with internal sequencing options.
There's also the general taste thing with an API that basically just
consists of passing a random value through - there's a lack of
generality there, it wouldn't be possible to write a generic user of the
API which is a bit of a warning sign.

If you just care about the specific "this pin controls enable in sleep
state" I'd suggest making an interface that very specifically does that.

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