* [PATCH 0/2] regulator: max77686/trats2: Disable some regulators in suspend
@ 2014-10-21 8:25 Krzysztof Kozlowski
2014-10-21 8:25 ` [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs Krzysztof Kozlowski
2014-10-21 8:25 ` [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators Krzysztof Kozlowski
0 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 8:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This patchset makes use of Javier Martinez Cainllas changes [1].
It allows disabling some of the regulators during suspend to RAM.
Javier's patchset is necessary only to test it.
[1] [PATCH v3 0/2] ARM: EXYNOS: Call regulator suspend prepare/finish
https://lkml.org/lkml/2014/10/20/545
Best regards,
Krzysztof
Krzysztof Kozlowski (2):
regulator: max77686: Implement suspend disable for some LDOs
ARM: dts: exynos4412-trats: Add suspend configuration for max77686
regulators
arch/arm/boot/dts/exynos4412-trats2.dts | 135 +++++++++++++++++++++++++-------
drivers/regulator/max77686.c | 19 +++++
2 files changed, 124 insertions(+), 30 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs
2014-10-21 8:25 [PATCH 0/2] regulator: max77686/trats2: Disable some regulators in suspend Krzysztof Kozlowski
@ 2014-10-21 8:25 ` Krzysztof Kozlowski
2014-10-21 9:10 ` Javier Martinez Canillas
2014-10-21 8:25 ` [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators Krzysztof Kozlowski
1 sibling, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 8:25 UTC (permalink / raw)
To: linux-arm-kernel
Some LDOs of Maxim 77686 PMIC support disabling during system suspend
(LDO{2,6,7,8,10,11,12,14,15,16}). This was already implemented as part
of set_suspend_mode function. In that case the mode was one of:
- disable,
- normal mode,
- low power mode.
However there are no bindings for setting the mode during suspend.
Implement a set_suspend_disable function for regulators supporting this.
This helps reducing energy consumption during system sleep.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
drivers/regulator/max77686.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index ef1af2debbd2..84603051f24d 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -123,6 +123,24 @@ static int max77686_set_suspend_mode(struct regulator_dev *rdev,
return 0;
}
+static int max77686_ldo_set_suspend_disable(struct regulator_dev *rdev,
+ unsigned int mode)
+{
+ unsigned int val;
+ struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+ int ret;
+
+ val = 0x1 << MAX77686_OPMODE_SHIFT;
+
+ ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+ rdev->desc->enable_mask, val);
+ if (ret)
+ return ret;
+
+ max77686->opmode[rdev_get_id(rdev)] = val;
+ return 0;
+}
+
/* Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state */
static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
unsigned int mode)
@@ -212,6 +230,7 @@ static struct regulator_ops max77686_ldo_ops = {
.set_voltage_sel = regulator_set_voltage_sel_regmap,
.set_voltage_time_sel = regulator_set_voltage_time_sel,
.set_suspend_mode = max77686_ldo_set_suspend_mode,
+ .set_suspend_disable = max77686_ldo_set_suspend_disable,
};
static struct regulator_ops max77686_buck1_ops = {
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 8:25 [PATCH 0/2] regulator: max77686/trats2: Disable some regulators in suspend Krzysztof Kozlowski
2014-10-21 8:25 ` [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs Krzysztof Kozlowski
@ 2014-10-21 8:25 ` Krzysztof Kozlowski
2014-10-21 8:44 ` Javier Martinez Canillas
2014-10-21 8:56 ` Chanwoo Choi
1 sibling, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 8:25 UTC (permalink / raw)
To: linux-arm-kernel
Add suspend to RAM configuration for max77686 regulators. Some LDOs and
bucks are disabled. This reduces energy consumption during S2R,
approximately from 17 mA to 9 mA.
Additionally remove old and not supported bindings:
- regulator-mem-off
- regulator-mem-idle
- regulator-mem-on
The max77686 driver does not parse them and they are not documented
anywere.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
arch/arm/boot/dts/exynos4412-trats2.dts | 135 +++++++++++++++++++++++++-------
1 file changed, 105 insertions(+), 30 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index dd9ac66770f7..c2fbee36021a 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -225,7 +225,9 @@
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo2_reg: ldo2 {
@@ -234,7 +236,9 @@
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo3_reg: ldo3 {
@@ -243,7 +247,9 @@
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo4_reg: ldo4 {
@@ -252,7 +258,9 @@
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo5_reg: ldo5 {
@@ -261,7 +269,9 @@
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo6_reg: ldo6 {
@@ -270,7 +280,9 @@
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo7_reg: ldo7 {
@@ -279,7 +291,9 @@
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-always-on;
- regulator-mem-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo8_reg: ldo8 {
@@ -287,7 +301,9 @@
regulator-name = "VMIPI_1.0V";
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo9_reg: ldo9 {
@@ -295,7 +311,9 @@
regulator-name = "CAM_ISP_MIPI_1.2V";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo10_reg: ldo10 {
@@ -303,7 +321,9 @@
regulator-name = "VMIPI_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo11_reg: ldo11 {
@@ -312,7 +332,9 @@
regulator-min-microvolt = <1950000>;
regulator-max-microvolt = <1950000>;
regulator-always-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo12_reg: ldo12 {
@@ -320,7 +342,9 @@
regulator-name = "VUOTG_3.0V";
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo13_reg: ldo13 {
@@ -328,7 +352,9 @@
regulator-name = "NFC_AVDD_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo14_reg: ldo14 {
@@ -337,7 +363,9 @@
regulator-min-microvolt = <1950000>;
regulator-max-microvolt = <1950000>;
regulator-always-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo15_reg: ldo15 {
@@ -345,7 +373,9 @@
regulator-name = "VHSIC_1.0V";
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo16_reg: ldo16 {
@@ -353,7 +383,9 @@
regulator-name = "VHSIC_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
ldo17_reg: ldo17 {
@@ -361,7 +393,9 @@
regulator-name = "CAM_SENSOR_CORE_1.2V";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo18_reg: ldo18 {
@@ -369,7 +403,9 @@
regulator-name = "CAM_ISP_SEN_IO_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo19_reg: ldo19 {
@@ -377,7 +413,9 @@
regulator-name = "VT_CAM_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo20_reg: ldo20 {
@@ -385,7 +423,9 @@
regulator-name = "VDDQ_PRE_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo21_reg: ldo21 {
@@ -393,7 +433,9 @@
regulator-name = "VTF_2.8V";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo22_reg: ldo22 {
@@ -401,6 +443,9 @@
regulator-name = "VMEM_VDD_2.8V";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo23_reg: ldo23 {
@@ -408,7 +453,9 @@
regulator-name = "TSP_AVDD_3.3V";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo24_reg: ldo24 {
@@ -416,7 +463,9 @@
regulator-name = "TSP_VDD_1.8V";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo25_reg: ldo25 {
@@ -424,7 +473,9 @@
regulator-name = "LCD_VCC_3.3V";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
ldo26_reg: ldo26 {
@@ -432,7 +483,9 @@
regulator-name = "MOTOR_VCC_3.0V";
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
- regulator-mem-idle;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck1_reg: buck1 {
@@ -442,7 +495,9 @@
regulator-max-microvolt = <1100000>;
regulator-always-on;
regulator-boot-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck2_reg: buck2 {
@@ -452,7 +507,9 @@
regulator-max-microvolt = <1500000>;
regulator-always-on;
regulator-boot-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck3_reg: buck3 {
@@ -462,7 +519,9 @@
regulator-max-microvolt = <1150000>;
regulator-always-on;
regulator-boot-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck4_reg: buck4 {
@@ -471,7 +530,9 @@
regulator-min-microvolt = <850000>;
regulator-max-microvolt = <1150000>;
regulator-boot-on;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck5_reg: buck5 {
@@ -480,6 +541,9 @@
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck6_reg: buck6 {
@@ -488,6 +552,9 @@
regulator-min-microvolt = <1350000>;
regulator-max-microvolt = <1350000>;
regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck7_reg: buck7 {
@@ -496,6 +563,9 @@
regulator-min-microvolt = <2000000>;
regulator-max-microvolt = <2000000>;
regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck8_reg: buck8 {
@@ -503,6 +573,9 @@
regulator-name = "VMEM_VDDF_3.0V";
regulator-min-microvolt = <2850000>;
regulator-max-microvolt = <2850000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck9_reg: buck9 {
@@ -510,7 +583,9 @@
regulator-name = "CAM_ISP_CORE_1.2V";
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1200000>;
- regulator-mem-off;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
};
};
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 8:25 ` [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators Krzysztof Kozlowski
@ 2014-10-21 8:44 ` Javier Martinez Canillas
2014-10-21 8:56 ` Chanwoo Choi
1 sibling, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2014-10-21 8:44 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
On 10/21/2014 10:25 AM, Krzysztof Kozlowski wrote:
> Add suspend to RAM configuration for max77686 regulators. Some LDOs and
> bucks are disabled. This reduces energy consumption during S2R,
> approximately from 17 mA to 9 mA.
>
> Additionally remove old and not supported bindings:
> - regulator-mem-off
> - regulator-mem-idle
> - regulator-mem-on
> The max77686 driver does not parse them and they are not documented
> anywere.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
> arch/arm/boot/dts/exynos4412-trats2.dts | 135 +++++++++++++++++++++++++-------
> 1 file changed, 105 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index dd9ac66770f7..c2fbee36021a 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Best regards,
Javier
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 8:25 ` [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators Krzysztof Kozlowski
2014-10-21 8:44 ` Javier Martinez Canillas
@ 2014-10-21 8:56 ` Chanwoo Choi
2014-10-21 9:23 ` Krzysztof Kozlowski
1 sibling, 1 reply; 11+ messages in thread
From: Chanwoo Choi @ 2014-10-21 8:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
I think that the state of some regulators should be changed in suspend state.
On 10/21/2014 05:25 PM, Krzysztof Kozlowski wrote:
> Add suspend to RAM configuration for max77686 regulators. Some LDOs and
> bucks are disabled. This reduces energy consumption during S2R,
> approximately from 17 mA to 9 mA.
>
> Additionally remove old and not supported bindings:
> - regulator-mem-off
> - regulator-mem-idle
> - regulator-mem-on
> The max77686 driver does not parse them and they are not documented
> anywere.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
> arch/arm/boot/dts/exynos4412-trats2.dts | 135 +++++++++++++++++++++++++-------
> 1 file changed, 105 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index dd9ac66770f7..c2fbee36021a 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -225,7 +225,9 @@
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1000000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo2_reg: ldo2 {
> @@ -234,7 +236,9 @@
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo3_reg: ldo3 {
> @@ -243,7 +247,9 @@
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo4_reg: ldo4 {
> @@ -252,7 +258,9 @@
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo5_reg: ldo5 {
> @@ -261,7 +269,9 @@
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo6_reg: ldo6 {
> @@ -270,7 +280,9 @@
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1000000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo7_reg: ldo7 {
> @@ -279,7 +291,9 @@
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1000000>;
> regulator-always-on;
> - regulator-mem-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo8_reg: ldo8 {
> @@ -287,7 +301,9 @@
> regulator-name = "VMIPI_1.0V";
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1000000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> ldo9_reg: ldo9 {
> @@ -295,7 +311,9 @@
> regulator-name = "CAM_ISP_MIPI_1.2V";
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
CAM_ISP_MIPI_1.2V is used for camera, I think this regulator should turn off in suspend state
because camear could not be used in suspend state.
> + };
> };
>
> ldo10_reg: ldo10 {
> @@ -303,7 +321,9 @@
> regulator-name = "VMIPI_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> ldo11_reg: ldo11 {
> @@ -312,7 +332,9 @@
> regulator-min-microvolt = <1950000>;
> regulator-max-microvolt = <1950000>;
> regulator-always-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> ldo12_reg: ldo12 {
> @@ -320,7 +342,9 @@
> regulator-name = "VUOTG_3.0V";
> regulator-min-microvolt = <3000000>;
> regulator-max-microvolt = <3000000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> ldo13_reg: ldo13 {
> @@ -328,7 +352,9 @@
> regulator-name = "NFC_AVDD_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
NFC_AVDD_1.8V is used for NFC feature, If user of smartphone want to use NFC feature
and then enable NFC on the setting menu, NFC_AVDD_1.8V should turn on in suspend state.
But, If NFC is not used, NFC_AVDD_1.8V should turn off in suspend state.
So, I think this regulator should be controlled by NFC device driver without 'regulator-state-mem' property.
> + };
> };
>
> ldo14_reg: ldo14 {
> @@ -337,7 +363,9 @@
> regulator-min-microvolt = <1950000>;
> regulator-max-microvolt = <1950000>;
> regulator-always-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> ldo15_reg: ldo15 {
> @@ -345,7 +373,9 @@
> regulator-name = "VHSIC_1.0V";
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1000000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
VHSIC_1.8V was used for CP (Modem), CP (Modem) have to maintain the power in suspend state.
- off -> on
> + };
> };
>
> ldo16_reg: ldo16 {
> @@ -353,7 +383,9 @@
> regulator-name = "VHSIC_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
ditto (off -> on) for CP (MODEM)
> + };
> };
>
> ldo17_reg: ldo17 {
> @@ -361,7 +393,9 @@
> regulator-name = "CAM_SENSOR_CORE_1.2V";
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
ditto. (on -> off) because camera is not used in supend state.
> + };
> };
>
> ldo18_reg: ldo18 {
> @@ -369,7 +403,9 @@
> regulator-name = "CAM_ISP_SEN_IO_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
ditto. (on -> off) because camera is not used in supend state.
> + };
> };
>
> ldo19_reg: ldo19 {
> @@ -377,7 +413,9 @@
> regulator-name = "VT_CAM_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
ditto. (on -> off) because camera is not used in supend state.
> + };
> };
>
> ldo20_reg: ldo20 {
> @@ -385,7 +423,9 @@
> regulator-name = "VDDQ_PRE_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo21_reg: ldo21 {
> @@ -393,7 +433,9 @@
> regulator-name = "VTF_2.8V";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo22_reg: ldo22 {
> @@ -401,6 +443,9 @@
> regulator-name = "VMEM_VDD_2.8V";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> ldo23_reg: ldo23 {
> @@ -408,7 +453,9 @@
> regulator-name = "TSP_AVDD_3.3V";
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3300000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
ditto. (on -> off) because touchscreen is not used in suspend state.
> + };
> };
>
> ldo24_reg: ldo24 {
> @@ -416,7 +463,9 @@
> regulator-name = "TSP_VDD_1.8V";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
ditto. (on -> off)
> + };
> };
>
> ldo25_reg: ldo25 {
> @@ -424,7 +473,9 @@
> regulator-name = "LCD_VCC_3.3V";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
LDC_VCC_3.3V may not be used in susepnd state. I think this regulator should turn off in suspend state.
> };
>
> ldo26_reg: ldo26 {
> @@ -432,7 +483,9 @@
> regulator-name = "MOTOR_VCC_3.0V";
> regulator-min-microvolt = <3000000>;
> regulator-max-microvolt = <3000000>;
> - regulator-mem-idle;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
If MOTOR_VCC_3.0V is used for haptic, I think this regulator should turn off in suspend state.
> };
>
> buck1_reg: buck1 {
> @@ -442,7 +495,9 @@
> regulator-max-microvolt = <1100000>;
> regulator-always-on;
> regulator-boot-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> buck2_reg: buck2 {
> @@ -452,7 +507,9 @@
> regulator-max-microvolt = <1500000>;
> regulator-always-on;
> regulator-boot-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> buck3_reg: buck3 {
> @@ -462,7 +519,9 @@
> regulator-max-microvolt = <1150000>;
> regulator-always-on;
> regulator-boot-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> buck4_reg: buck4 {
> @@ -471,7 +530,9 @@
> regulator-min-microvolt = <850000>;
> regulator-max-microvolt = <1150000>;
> regulator-boot-on;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
>
> buck5_reg: buck5 {
> @@ -480,6 +541,9 @@
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> regulator-always-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> buck6_reg: buck6 {
> @@ -488,6 +552,9 @@
> regulator-min-microvolt = <1350000>;
> regulator-max-microvolt = <1350000>;
> regulator-always-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> buck7_reg: buck7 {
> @@ -496,6 +563,9 @@
> regulator-min-microvolt = <2000000>;
> regulator-max-microvolt = <2000000>;
> regulator-always-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> buck8_reg: buck8 {
> @@ -503,6 +573,9 @@
> regulator-name = "VMEM_VDDF_3.0V";
> regulator-min-microvolt = <2850000>;
> regulator-max-microvolt = <2850000>;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> };
>
> buck9_reg: buck9 {
> @@ -510,7 +583,9 @@
> regulator-name = "CAM_ISP_CORE_1.2V";
> regulator-min-microvolt = <1000000>;
> regulator-max-microvolt = <1200000>;
> - regulator-mem-off;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> };
> };
> };
>
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs
2014-10-21 8:25 ` [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs Krzysztof Kozlowski
@ 2014-10-21 9:10 ` Javier Martinez Canillas
2014-10-21 9:36 ` Krzysztof Kozlowski
0 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2014-10-21 9:10 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
On 10/21/2014 10:25 AM, Krzysztof Kozlowski wrote:
> Some LDOs of Maxim 77686 PMIC support disabling during system suspend
> (LDO{2,6,7,8,10,11,12,14,15,16}). This was already implemented as part
You should also document what each regulator support in the max77686 DT binding
doc, which btw I see is in Documentation/devicetree/bindings/mfd/max77686.txt
while I think it should be in Documentation/devicetree/bindings/regulators/
I just posted a patch to add this information to the max77802 DT binding doc [0]
so maybe you can add something along those lines?
> of set_suspend_mode function. In that case the mode was one of:
> - disable,
> - normal mode,
> - low power mode.
> However there are no bindings for setting the mode during suspend.
>
> Implement a set_suspend_disable function for regulators supporting this.
> This helps reducing energy consumption during system sleep.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
> drivers/regulator/max77686.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
> index ef1af2debbd2..84603051f24d 100644
> --- a/drivers/regulator/max77686.c
> +++ b/drivers/regulator/max77686.c
> @@ -123,6 +123,24 @@ static int max77686_set_suspend_mode(struct regulator_dev *rdev,
> return 0;
> }
>
> +static int max77686_ldo_set_suspend_disable(struct regulator_dev *rdev,
> + unsigned int mode)
> +{
> + unsigned int val;
> + struct max77686_data *max77686 = rdev_get_drvdata(rdev);
> + int ret;
> +
> + val = 0x1 << MAX77686_OPMODE_SHIFT;
Maybe instead of using magic numbers you can document what 0x1 means here.
I don't have access to the max77686 data-sheet but I do have for the max77802
so my educated guess is that 0x1 means "Output ON/OFF Controlled by PWRREQ"
and that the Exynos 4412 XPWRRGTON pin is connected to the max77686 PWRREQ in
the Trats2 board.
> +
> + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> + rdev->desc->enable_mask, val);
> + if (ret)
> + return ret;
> +
> + max77686->opmode[rdev_get_id(rdev)] = val;
> + return 0;
> +}
> +
There is already a max77686_buck_set_suspend_disable() that is used by BUCK 1-4.
At least in the max77802, all regulators expect LDO 1, 3, 20 and 21 support OFF
by PWRREQ so with some refactoring (using a function to get the opmode shift)
you can use the same function for all regulators that support this instead of
having separate .set_suspend_disable function handlers.
> /* Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state */
> static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
> unsigned int mode)
> @@ -212,6 +230,7 @@ static struct regulator_ops max77686_ldo_ops = {
> .set_voltage_sel = regulator_set_voltage_sel_regmap,
> .set_voltage_time_sel = regulator_set_voltage_time_sel,
> .set_suspend_mode = max77686_ldo_set_suspend_mode,
> + .set_suspend_disable = max77686_ldo_set_suspend_disable,
You probably want add .set_suspend_enable as well.
> };
>
> static struct regulator_ops max77686_buck1_ops = {
>
In general, could you please take a look to the latest patches I posted for the
max77802? They are in a topic branch in Mark's regulator tree [1]. I would like
both 77686 and 77802 drivers to handle things similarly even though I know that
the PMICs don't behave identically and that's why we decided to have different
drivers (although maybe that was a mistake and makes more sense to merge them).
Thanks a lot and best regards,
Javier
[0]: https://lkml.org/lkml/2014/10/20/320
[1]: https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=topic/max77802
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 8:56 ` Chanwoo Choi
@ 2014-10-21 9:23 ` Krzysztof Kozlowski
2014-10-21 10:00 ` Javier Martinez Canillas
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 9:23 UTC (permalink / raw)
To: linux-arm-kernel
On wto, 2014-10-21 at 17:56 +0900, Chanwoo Choi wrote:
> Hi Krzysztof,
>
> I think that the state of some regulators should be changed in suspend state.
>
> On 10/21/2014 05:25 PM, Krzysztof Kozlowski wrote:
> > Add suspend to RAM configuration for max77686 regulators. Some LDOs and
> > bucks are disabled. This reduces energy consumption during S2R,
> > approximately from 17 mA to 9 mA.
> >
> > Additionally remove old and not supported bindings:
> > - regulator-mem-off
> > - regulator-mem-idle
> > - regulator-mem-on
> > The max77686 driver does not parse them and they are not documented
> > anywere.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos4412-trats2.dts | 135 +++++++++++++++++++++++++-------
> > 1 file changed, 105 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> > index dd9ac66770f7..c2fbee36021a 100644
> > --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> > +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> > @@ -225,7 +225,9 @@
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1000000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo2_reg: ldo2 {
> > @@ -234,7 +236,9 @@
> > regulator-min-microvolt = <1200000>;
> > regulator-max-microvolt = <1200000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo3_reg: ldo3 {
> > @@ -243,7 +247,9 @@
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo4_reg: ldo4 {
> > @@ -252,7 +258,9 @@
> > regulator-min-microvolt = <2800000>;
> > regulator-max-microvolt = <2800000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo5_reg: ldo5 {
> > @@ -261,7 +269,9 @@
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo6_reg: ldo6 {
> > @@ -270,7 +280,9 @@
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1000000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo7_reg: ldo7 {
> > @@ -279,7 +291,9 @@
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1000000>;
> > regulator-always-on;
> > - regulator-mem-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo8_reg: ldo8 {
> > @@ -287,7 +301,9 @@
> > regulator-name = "VMIPI_1.0V";
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1000000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > ldo9_reg: ldo9 {
> > @@ -295,7 +311,9 @@
> > regulator-name = "CAM_ISP_MIPI_1.2V";
> > regulator-min-microvolt = <1200000>;
> > regulator-max-microvolt = <1200000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> CAM_ISP_MIPI_1.2V is used for camera, I think this regulator should turn off in suspend state
> because camear could not be used in suspend state.
LDO9 does not support off in suspend mode controlled by PWRREQ. It
supports only switching to low power mode if PWRREQ level goes low.
Thus the driver does not provide set_suspend_enable or
set_suspend_disable for LDO0. Do you want to add such function to the
max77686 driver which will explicitly disable the regulator (write 0x0
to control reg)?
> > + };
> > };
> >
> > ldo10_reg: ldo10 {
> > @@ -303,7 +321,9 @@
> > regulator-name = "VMIPI_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > ldo11_reg: ldo11 {
> > @@ -312,7 +332,9 @@
> > regulator-min-microvolt = <1950000>;
> > regulator-max-microvolt = <1950000>;
> > regulator-always-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > ldo12_reg: ldo12 {
> > @@ -320,7 +342,9 @@
> > regulator-name = "VUOTG_3.0V";
> > regulator-min-microvolt = <3000000>;
> > regulator-max-microvolt = <3000000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > ldo13_reg: ldo13 {
> > @@ -328,7 +352,9 @@
> > regulator-name = "NFC_AVDD_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> NFC_AVDD_1.8V is used for NFC feature, If user of smartphone want to use NFC feature
> and then enable NFC on the setting menu, NFC_AVDD_1.8V should turn on in suspend state.
>
> But, If NFC is not used, NFC_AVDD_1.8V should turn off in suspend state.
> So, I think this regulator should be controlled by NFC device driver without 'regulator-state-mem' property.
Currently there is no API for regulator consumers for managing the
suspend state. Consumer cannot change suspend mode or suspend state for
regulator.
>
> > + };
> > };
> >
> > ldo14_reg: ldo14 {
> > @@ -337,7 +363,9 @@
> > regulator-min-microvolt = <1950000>;
> > regulator-max-microvolt = <1950000>;
> > regulator-always-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > ldo15_reg: ldo15 {
> > @@ -345,7 +373,9 @@
> > regulator-name = "VHSIC_1.0V";
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1000000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
>
> VHSIC_1.8V was used for CP (Modem), CP (Modem) have to maintain the power in suspend state.
> - off -> on
OK.
>
> > + };
> > };
> >
> > ldo16_reg: ldo16 {
> > @@ -353,7 +383,9 @@
> > regulator-name = "VHSIC_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
>
> ditto (off -> on) for CP (MODEM)
OK.
>
> > + };
> > };
> >
> > ldo17_reg: ldo17 {
> > @@ -361,7 +393,9 @@
> > regulator-name = "CAM_SENSOR_CORE_1.2V";
> > regulator-min-microvolt = <1200000>;
> > regulator-max-microvolt = <1200000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> ditto. (on -> off) because camera is not used in supend state.
LDO17 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > + };
> > };
> >
> > ldo18_reg: ldo18 {
> > @@ -369,7 +403,9 @@
> > regulator-name = "CAM_ISP_SEN_IO_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> ditto. (on -> off) because camera is not used in supend state.
LDO18 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > + };
> > };
> >
> > ldo19_reg: ldo19 {
> > @@ -377,7 +413,9 @@
> > regulator-name = "VT_CAM_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> ditto. (on -> off) because camera is not used in supend state.
LDO19 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > + };
> > };
> >
> > ldo20_reg: ldo20 {
> > @@ -385,7 +423,9 @@
> > regulator-name = "VDDQ_PRE_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo21_reg: ldo21 {
> > @@ -393,7 +433,9 @@
> > regulator-name = "VTF_2.8V";
> > regulator-min-microvolt = <2800000>;
> > regulator-max-microvolt = <2800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo22_reg: ldo22 {
> > @@ -401,6 +443,9 @@
> > regulator-name = "VMEM_VDD_2.8V";
> > regulator-min-microvolt = <2800000>;
> > regulator-max-microvolt = <2800000>;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > ldo23_reg: ldo23 {
> > @@ -408,7 +453,9 @@
> > regulator-name = "TSP_AVDD_3.3V";
> > regulator-min-microvolt = <3300000>;
> > regulator-max-microvolt = <3300000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> ditto. (on -> off) because touchscreen is not used in suspend state.
LDO23 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > + };
> > };
> >
> > ldo24_reg: ldo24 {
> > @@ -416,7 +463,9 @@
> > regulator-name = "TSP_VDD_1.8V";
> > regulator-min-microvolt = <1800000>;
> > regulator-max-microvolt = <1800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
>
> ditto. (on -> off)
LDO24 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > + };
> > };
> >
> > ldo25_reg: ldo25 {
> > @@ -424,7 +473,9 @@
> > regulator-name = "LCD_VCC_3.3V";
> > regulator-min-microvolt = <2800000>;
> > regulator-max-microvolt = <2800000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
>
> LDC_VCC_3.3V may not be used in susepnd state. I think this regulator should turn off in suspend state.
LDO25 does not support off mode controlled by PWRREQ. Same as for LDO9.
>
> > };
> >
> > ldo26_reg: ldo26 {
> > @@ -432,7 +483,9 @@
> > regulator-name = "MOTOR_VCC_3.0V";
> > regulator-min-microvolt = <3000000>;
> > regulator-max-microvolt = <3000000>;
> > - regulator-mem-idle;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
>
> If MOTOR_VCC_3.0V is used for haptic, I think this regulator should turn off in suspend state.
LDO26 does not support off mode controlled by PWRREQ. Same as for LDO9.
Thanks for feedback!
Best regards,
Krzysztof
>
> > };
> >
> > buck1_reg: buck1 {
> > @@ -442,7 +495,9 @@
> > regulator-max-microvolt = <1100000>;
> > regulator-always-on;
> > regulator-boot-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > buck2_reg: buck2 {
> > @@ -452,7 +507,9 @@
> > regulator-max-microvolt = <1500000>;
> > regulator-always-on;
> > regulator-boot-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > buck3_reg: buck3 {
> > @@ -462,7 +519,9 @@
> > regulator-max-microvolt = <1150000>;
> > regulator-always-on;
> > regulator-boot-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > buck4_reg: buck4 {
> > @@ -471,7 +530,9 @@
> > regulator-min-microvolt = <850000>;
> > regulator-max-microvolt = <1150000>;
> > regulator-boot-on;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> >
> > buck5_reg: buck5 {
> > @@ -480,6 +541,9 @@
> > regulator-min-microvolt = <1200000>;
> > regulator-max-microvolt = <1200000>;
> > regulator-always-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > buck6_reg: buck6 {
> > @@ -488,6 +552,9 @@
> > regulator-min-microvolt = <1350000>;
> > regulator-max-microvolt = <1350000>;
> > regulator-always-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > buck7_reg: buck7 {
> > @@ -496,6 +563,9 @@
> > regulator-min-microvolt = <2000000>;
> > regulator-max-microvolt = <2000000>;
> > regulator-always-on;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > buck8_reg: buck8 {
> > @@ -503,6 +573,9 @@
> > regulator-name = "VMEM_VDDF_3.0V";
> > regulator-min-microvolt = <2850000>;
> > regulator-max-microvolt = <2850000>;
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + };
> > };
> >
> > buck9_reg: buck9 {
> > @@ -510,7 +583,9 @@
> > regulator-name = "CAM_ISP_CORE_1.2V";
> > regulator-min-microvolt = <1000000>;
> > regulator-max-microvolt = <1200000>;
> > - regulator-mem-off;
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > };
> > };
> > };
> >
>
> Thanks,
> Chanwoo Choi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs
2014-10-21 9:10 ` Javier Martinez Canillas
@ 2014-10-21 9:36 ` Krzysztof Kozlowski
2014-10-21 9:54 ` Javier Martinez Canillas
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 9:36 UTC (permalink / raw)
To: linux-arm-kernel
On wto, 2014-10-21 at 11:10 +0200, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>
> On 10/21/2014 10:25 AM, Krzysztof Kozlowski wrote:
> > Some LDOs of Maxim 77686 PMIC support disabling during system suspend
> > (LDO{2,6,7,8,10,11,12,14,15,16}). This was already implemented as part
>
> You should also document what each regulator support in the max77686 DT binding
> doc, which btw I see is in Documentation/devicetree/bindings/mfd/max77686.txt
> while I think it should be in Documentation/devicetree/bindings/regulators/
> I just posted a patch to add this information to the max77802 DT binding doc [0]
> so maybe you can add something along those lines?
Sure, I'll document this.
> > of set_suspend_mode function. In that case the mode was one of:
> > - disable,
> > - normal mode,
> > - low power mode.
> > However there are no bindings for setting the mode during suspend.
> >
> > Implement a set_suspend_disable function for regulators supporting this.
> > This helps reducing energy consumption during system sleep.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> > drivers/regulator/max77686.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
> > index ef1af2debbd2..84603051f24d 100644
> > --- a/drivers/regulator/max77686.c
> > +++ b/drivers/regulator/max77686.c
> > @@ -123,6 +123,24 @@ static int max77686_set_suspend_mode(struct regulator_dev *rdev,
> > return 0;
> > }
> >
> > +static int max77686_ldo_set_suspend_disable(struct regulator_dev *rdev,
> > + unsigned int mode)
> > +{
> > + unsigned int val;
> > + struct max77686_data *max77686 = rdev_get_drvdata(rdev);
> > + int ret;
> > +
> > + val = 0x1 << MAX77686_OPMODE_SHIFT;
>
> Maybe instead of using magic numbers you can document what 0x1 means here.
> I don't have access to the max77686 data-sheet but I do have for the max77802
> so my educated guess is that 0x1 means "Output ON/OFF Controlled by PWRREQ"
> and that the Exynos 4412 XPWRRGTON pin is connected to the max77686 PWRREQ in
> the Trats2 board.
OK.
>
> > +
> > + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> > + rdev->desc->enable_mask, val);
> > + if (ret)
> > + return ret;
> > +
> > + max77686->opmode[rdev_get_id(rdev)] = val;
> > + return 0;
> > +}
> > +
>
> There is already a max77686_buck_set_suspend_disable() that is used by BUCK 1-4.
>
> At least in the max77802, all regulators expect LDO 1, 3, 20 and 21 support OFF
> by PWRREQ so with some refactoring (using a function to get the opmode shift)
> you can use the same function for all regulators that support this instead of
> having separate .set_suspend_disable function handlers.
OK, I can merge this into one suspend_disable.
>
> > /* Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state */
> > static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
> > unsigned int mode)
> > @@ -212,6 +230,7 @@ static struct regulator_ops max77686_ldo_ops = {
> > .set_voltage_sel = regulator_set_voltage_sel_regmap,
> > .set_voltage_time_sel = regulator_set_voltage_time_sel,
> > .set_suspend_mode = max77686_ldo_set_suspend_mode,
> > + .set_suspend_disable = max77686_ldo_set_suspend_disable,
>
> You probably want add .set_suspend_enable as well.
Yes, I'll add it.
>
> > };
> >
> > static struct regulator_ops max77686_buck1_ops = {
> >
>
> In general, could you please take a look to the latest patches I posted for the
> max77802? They are in a topic branch in Mark's regulator tree [1]. I would like
> both 77686 and 77802 drivers to handle things similarly even though I know that
> the PMICs don't behave identically and that's why we decided to have different
> drivers (although maybe that was a mistake and makes more sense to merge them).
I looked at them. In this patchset I didn't want to implement fully the
whole suspend-mode-thing. Just the first, easier part - set suspend
disable.
In general I would like to do it similar to your solution but that is
work for future. Especially as I would like to wait for merging your
"Add max77802 regulator operating mode support".
Best regards,
Krzysztof
>
> Thanks a lot and best regards,
> Javier
>
> [0]: https://lkml.org/lkml/2014/10/20/320
> [1]: https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=topic/max77802
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs
2014-10-21 9:36 ` Krzysztof Kozlowski
@ 2014-10-21 9:54 ` Javier Martinez Canillas
0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2014-10-21 9:54 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
On 10/21/2014 11:36 AM, Krzysztof Kozlowski wrote:
> On wto, 2014-10-21 at 11:10 +0200, Javier Martinez Canillas wrote:
>>
>> In general, could you please take a look to the latest patches I posted for the
>> max77802? They are in a topic branch in Mark's regulator tree [1]. I would like
>> both 77686 and 77802 drivers to handle things similarly even though I know that
>> the PMICs don't behave identically and that's why we decided to have different
>> drivers (although maybe that was a mistake and makes more sense to merge them).
>
> I looked at them. In this patchset I didn't want to implement fully the
> whole suspend-mode-thing. Just the first, easier part - set suspend
> disable.
>
Agreed, incremental support makes sense to me as well.
I also want to configure the regulator modes in suspend for the Snow Chromebook
using the same modes that are in the downstream Chrome OS tree and I think your
changes will be enough for that too.
> In general I would like to do it similar to your solution but that is
> work for future. Especially as I would like to wait for merging your
> "Add max77802 regulator operating mode support".
>
Perfect, thanks a lot for working on this!
> Best regards,
> Krzysztof
>
Best regards,
Javier
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 9:23 ` Krzysztof Kozlowski
@ 2014-10-21 10:00 ` Javier Martinez Canillas
2014-10-21 10:14 ` Krzysztof Kozlowski
0 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2014-10-21 10:00 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
On 10/21/2014 11:23 AM, Krzysztof Kozlowski wrote:
>> >
>> > ldo9_reg: ldo9 {
>> > @@ -295,7 +311,9 @@
>> > regulator-name = "CAM_ISP_MIPI_1.2V";
>> > regulator-min-microvolt = <1200000>;
>> > regulator-max-microvolt = <1200000>;
>> > - regulator-mem-idle;
>> > + regulator-state-mem {
>> > + regulator-on-in-suspend;
>>
>> CAM_ISP_MIPI_1.2V is used for camera, I think this regulator should turn off in suspend state
>> because camear could not be used in suspend state.
>
> LDO9 does not support off in suspend mode controlled by PWRREQ. It
> supports only switching to low power mode if PWRREQ level goes low.
>
> Thus the driver does not provide set_suspend_enable or
> set_suspend_disable for LDO0. Do you want to add such function to the
> max77686 driver which will explicitly disable the regulator (write 0x0
> to control reg)?
>
>
I think for those regulators you can just use regulator_disable_regmap()
as the .set_suspend_disable function handler?
Best regards,
Javier
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators
2014-10-21 10:00 ` Javier Martinez Canillas
@ 2014-10-21 10:14 ` Krzysztof Kozlowski
0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2014-10-21 10:14 UTC (permalink / raw)
To: linux-arm-kernel
On wto, 2014-10-21 at 12:00 +0200, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>
> On 10/21/2014 11:23 AM, Krzysztof Kozlowski wrote:
> >> >
> >> > ldo9_reg: ldo9 {
> >> > @@ -295,7 +311,9 @@
> >> > regulator-name = "CAM_ISP_MIPI_1.2V";
> >> > regulator-min-microvolt = <1200000>;
> >> > regulator-max-microvolt = <1200000>;
> >> > - regulator-mem-idle;
> >> > + regulator-state-mem {
> >> > + regulator-on-in-suspend;
> >>
> >> CAM_ISP_MIPI_1.2V is used for camera, I think this regulator should turn off in suspend state
> >> because camear could not be used in suspend state.
> >
> > LDO9 does not support off in suspend mode controlled by PWRREQ. It
> > supports only switching to low power mode if PWRREQ level goes low.
> >
> > Thus the driver does not provide set_suspend_enable or
> > set_suspend_disable for LDO0. Do you want to add such function to the
> > max77686 driver which will explicitly disable the regulator (write 0x0
> > to control reg)?
> >
> >
>
> I think for those regulators you can just use regulator_disable_regmap()
> as the .set_suspend_disable function handler?
Sure... but this does not solve the philosophical problem - this is not
a special suspend configuration. It is disabling the regulator manually,
from regulator core code. Instead probably the camera driver should
disable the regulator in suspend callback.
Another point - I think none of other regulators do this way.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-10-21 10:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-21 8:25 [PATCH 0/2] regulator: max77686/trats2: Disable some regulators in suspend Krzysztof Kozlowski
2014-10-21 8:25 ` [PATCH 1/2] regulator: max77686: Implement suspend disable for some LDOs Krzysztof Kozlowski
2014-10-21 9:10 ` Javier Martinez Canillas
2014-10-21 9:36 ` Krzysztof Kozlowski
2014-10-21 9:54 ` Javier Martinez Canillas
2014-10-21 8:25 ` [PATCH 2/2] ARM: dts: exynos4412-trats: Add suspend configuration for max77686 regulators Krzysztof Kozlowski
2014-10-21 8:44 ` Javier Martinez Canillas
2014-10-21 8:56 ` Chanwoo Choi
2014-10-21 9:23 ` Krzysztof Kozlowski
2014-10-21 10:00 ` Javier Martinez Canillas
2014-10-21 10:14 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).