| Message ID | 1413478133-2577-6-git-send-email-javier.martinez@collabora.co.uk |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
On Thu, Oct 16, 2014 at 06:48:51PM +0200, Javier Martinez Canillas wrote: > +- maxim,regulator-initial-mode: initial operating mode. > + This property can only be used on regulators that support changing their mode > + during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21. > +- maxim,regulator-disk-mode: operating mode for the regulator when the system > + enters in the Suspend-to-Disk state. > +- maxim,regulator-mem-mode: operating mode for the regulator when the system > + enters in the Suspend-to-RAM state. This seems pretty ugly since it's not integrated with the suspend state binding at all - adding new suspend modes is going to involve changing the binding which seems icky. Adding a standard property to set modes doesn't seem so bad, I think a translation function to parse device specific mode bindings in properties might be the way forwards.
Hello Mark, On 10/17/2014 01:57 PM, Mark Brown wrote: > On Thu, Oct 16, 2014 at 06:48:51PM +0200, Javier Martinez Canillas wrote: > >> +- maxim,regulator-initial-mode: initial operating mode. >> + This property can only be used on regulators that support changing their mode >> + during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21. >> +- maxim,regulator-disk-mode: operating mode for the regulator when the system >> + enters in the Suspend-to-Disk state. >> +- maxim,regulator-mem-mode: operating mode for the regulator when the system >> + enters in the Suspend-to-RAM state. > > This seems pretty ugly since it's not integrated with the suspend state > binding at all - adding new suspend modes is going to involve changing > the binding which seems icky. Adding a standard property to set modes > doesn't seem so bad, I think a translation function to parse device > specific mode bindings in properties might be the way forwards. > Just to be sure I understood correctly, are you suggesting something like this? ldo1_reg: LDO1 { regulator-name = "vdd_1v0"; regulator-min-microvolt = <1000000>; regulator-max-microvolt = <1000000>; regulator-state-mem { regulator-on-in-suspend; regulator-mode = <MAX77802_OPMODE_LP>; }; }; In other words, extending Chanwoo Choi's original suspend state binding to add the regulator-mode property that was present in his v3 [0] but instead trying to use the standard REGULATOR_MODE_*, say that each regulator driver should define it's own device-specific set of modes and a do the translation to fill standard modes in the struct regulation_constraints {initial,disk,mem} mode? That way adding new suspend states, will only require changing the generic regulator binding but not the regulator driver specific bindings. Best regards, Javier [0]: https://lkml.org/lkml/2014/8/13/768 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote: > Just to be sure I understood correctly, are you suggesting something like this? > ldo1_reg: LDO1 { > regulator-name = "vdd_1v0"; > regulator-min-microvolt = <1000000>; > regulator-max-microvolt = <1000000>; > regulator-state-mem { > regulator-on-in-suspend; > regulator-mode = <MAX77802_OPMODE_LP>; > }; > }; > In other words, extending Chanwoo Choi's original suspend state binding to add > the regulator-mode property that was present in his v3 [0] but instead trying > to use the standard REGULATOR_MODE_*, say that each regulator driver should > define it's own device-specific set of modes and a do the translation to fill > standard modes in the struct regulation_constraints {initial,disk,mem} mode? > That way adding new suspend states, will only require changing the generic > regulator binding but not the regulator driver specific bindings. Something like that, yes. Not sure if numbers or strings are the best way of doing the mode but it probably doesn't matter too much now we have preprocessor support for inclue files.
Hello Mark, On 10/17/2014 03:54 PM, Mark Brown wrote: > On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote: > >> Just to be sure I understood correctly, are you suggesting something like this? > >> ldo1_reg: LDO1 { >> regulator-name = "vdd_1v0"; >> regulator-min-microvolt = <1000000>; >> regulator-max-microvolt = <1000000>; >> regulator-state-mem { >> regulator-on-in-suspend; >> regulator-mode = <MAX77802_OPMODE_LP>; >> }; >> }; > >> In other words, extending Chanwoo Choi's original suspend state binding to add >> the regulator-mode property that was present in his v3 [0] but instead trying >> to use the standard REGULATOR_MODE_*, say that each regulator driver should >> define it's own device-specific set of modes and a do the translation to fill >> standard modes in the struct regulation_constraints {initial,disk,mem} mode? > >> That way adding new suspend states, will only require changing the generic >> regulator binding but not the regulator driver specific bindings. > > Something like that, yes. Not sure if numbers or strings are the best Perfect will re-spin then, many thanks again for your feedback and suggestions. > way of doing the mode but it probably doesn't matter too much now we > have preprocessor support for inclue files. > I usually prefer to avoid strings when possible since a typo can't be detected when building the DTB and could be hard to debug at runtime while a typo on a macro will be detected by the preprocessor at build time. But I don't have a strong opinion either. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt index 5aeaffc..34812e0 100644 --- a/Documentation/devicetree/bindings/regulator/max77802.txt +++ b/Documentation/devicetree/bindings/regulator/max77802.txt @@ -25,6 +25,39 @@ with their hardware counterparts as follow. The valid names are: example: LDO1, LDO2, LDO35. -BUCKn : for BUCKs, where n can lie in range 1 to 10. example: BUCK1, BUCK5, BUCK10. + +Besides the standard regulator constraints, the max77802 regulator supports +two different operating modes: Normal and Low Power Mode. Some regulators +support the modes to be changed at startup or by the consumers during normal +operation while others only support to change the mode during system suspend. +The following optional properties can be used to configure the operating modes: + +- maxim,regulator-initial-mode: initial operating mode. + This property can only be used on regulators that support changing their mode + during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21. +- maxim,regulator-disk-mode: operating mode for the regulator when the system + enters in the Suspend-to-Disk state. +- maxim,regulator-mem-mode: operating mode for the regulator when the system + enters in the Suspend-to-RAM state. + +The value for maxim,regulator-[initial,disk,mem]-mode is one of the following: + 1: Normal regulator voltage output mode. + 3: Low Power which reduces the quiescent current down to only 1uA + +The list of valid modes are defined in the dt-bindings/regulator/regulator.h +header and can be included by device tree source files. If no mode is defined, +then the OS will not manage the modes and the HW default values will be used. + +The maxim,regulator-[initial,disk,mem]-mode properties can only be used with +regulators that support changing their mode to Low Power Mode during suspend. +These regulators are BUCKs 2-4 and LDOs 1-35. + +The maxim,regulator-[disk,mem]-mode property only takes effect if the regulator +has been marked as enabled for the given suspend mode using the standard +"regulator-on-in-suspend" property. If the regulator has not been explicitly +enabled or if it was marked as disabled with "regulator-off-in-suspend", then +setting the operating mode for that state will have no effect. + Example: max77802@09 { @@ -36,11 +69,23 @@ Example: #size-cells = <0>; regulators { + ldo1_reg: LDO1 { + regulator-name = "vdd_1v0"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + maxim,regulator-initial-mode = <MAX77802_OPMODE_LP>; + }; + ldo11_reg: LDO11 { regulator-name = "vdd_ldo11"; regulator-min-microvolt = <1900000>; regulator-max-microvolt = <1900000>; regulator-always-on; + maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>; + regulator-state-mem { + regulator-on-in-suspend; + }; }; buck1_reg: BUCK1 {
Some regulators from the max77802 PMIC support to be configured in one of two operating mode: Output ON (normal) and Output On Low Power Mode. Not all regulators support these two modes and for some of them, the mode can be changed while the system is running in normal operation while others only support their mode to be changed on system suspend. Extend the max77802 PMIC binding adding Device Tree properties so the regulators operating modes can be configured. Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> --- Changes since v1: None .../devicetree/bindings/regulator/max77802.txt | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+)