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]

Date:	Thu, 09 Oct 2014 23:40:17 +0200
From:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
To:	Mark Brown <broonie@...nel.org>
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

Hello Mark,

Thanks for your feedback.

On 10/09/2014 07:48 PM, Mark Brown wrote:
> 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...
> 

Sorry, I didn't mean that you are not correct but more that I was wrong
on my assumptions.

>> 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?
> 

Right, I got confused again by the terminology in the Maxim data-sheet that
list output OFF as an opmode but I understand that OFF is not a mode and
that the regulator API treats it separately.

>> 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.
> 

Yes, I'll not try to make it generic and will do something that is specific
to this device.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/