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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/