[PATCH v3 0/2] ARM: EXYNOS: Call regulator suspend prepare/finish

[PATCH v3 0/2] ARM: EXYNOS: Call regulator suspend prepare/finish

[Javier Martinez Canillas]

Hello,

This series add support for Exynos platforms to prepare regulators for
system suspend. The regulator core has a set of helpers functions to be
used when the system is entering and leaving from a suspend state but
currently there is only one user in mainline.

This user is drivers/mfd/sec-core.c but it calls regulator_suspend_prepare()
from within the driver power-management suspend function. This does not
seems to be correct since the regulator suspend prepare function affects all
regulators in the system and not only the ones managed by this device.

So patch #1 in this series revert the commit that introduced that change and
patch #2 calls the regulator framework suspend/finish functions from the
Exynos platform power-management code. The first patch should be queued through
the mfd tree and the second through the linux-samsung tree.

Changes since v2:
 - Check for regulator_suspend_finish() return value for an error.
   Suggested by Doug Anderson.

Changes since v1:
 - Remove the call to regulator_suspend_prepare() from drivers/mfd/sec-core.c
   as suggested by Doug Anderson.
 - Call regulator_suspend_prepare() before s3c_pm_check_prepare() as suggested
   by Doug Anderson.
 - Added Lee Jones to cc list since there is a change for the mfd framework.

Javier Martinez Canillas (2):
  Revert "mfd: sec-core: Prepare regulators for suspend state to reduce
    power-consumption"
  ARM: EXYNOS: Call regulator core suspend prepare and finish functions

 arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
 drivers/mfd/Kconfig            |  1 -
 drivers/mfd/sec-core.c         | 10 ----------
 3 files changed, 23 insertions(+), 11 deletions(-)

-- 
2.1.0

[PATCH v3 1/2] Revert "mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption"

[Javier Martinez Canillas]

This reverts commit b7cde7078d2344073c310aa65fc2b0a845d2cb5b
("mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption")

Commit b7cde7078d23 called regulator_suspend_prepare() to prepare the
regulators for a suspend state. But it did from the device pm suspend
handler while the regulator suspend prepare function iterates over all
regulators and not only the one managed by this device so it doesn't
seems to be correct to call it from within a device driver.

It is better to call the regulator suspend prepare/finish functions
from platform code instead so this patch reverts the mentioned commit.

Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/Kconfig    |  1 -
 drivers/mfd/sec-core.c | 10 ----------
 2 files changed, 11 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1456ea7..fd8cc4c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -655,7 +655,6 @@ config MFD_SEC_CORE
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-	select REGULATOR
 	help
 	 Support for the Samsung Electronics MFD series.
 	 This driver provides common support for accessing the device,
diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index dba7e2b..5993608 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -31,7 +31,6 @@
 #include <linux/mfd/samsung/s2mpu02.h>
 #include <linux/mfd/samsung/s5m8763.h>
 #include <linux/mfd/samsung/s5m8767.h>
-#include <linux/regulator/machine.h>
 #include <linux/regmap.h>
 
 static const struct mfd_cell s5m8751_devs[] = {
@@ -432,15 +431,6 @@ static int sec_pmic_suspend(struct device *dev)
 	 */
 	disable_irq(sec_pmic->irq);
 
-	switch (sec_pmic->device_type) {
-	case S2MPS14X:
-	case S2MPU02:
-		regulator_suspend_prepare(PM_SUSPEND_MEM);
-		break;
-	default:
-		break;
-	}
-
 	return 0;
 }
 
-- 
2.1.0

[PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Javier Martinez Canillas]

The regulator framework has a set of helpers functions to be used when
the system is entering and leaving from suspend but these are not called
on Exynos platforms. This means that the .set_suspend_* function handlers
defined by regulator drivers are not called when the system is suspended.

Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Doug Anderson <dianders@chromium.org>
---
 arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index cc8d237..f8e7dcd 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/irqchip/arm-gic.h>
 #include <linux/err.h>
+#include <linux/regulator/machine.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -443,6 +444,22 @@ static int exynos_suspend_enter(suspend_state_t state)
 
 static int exynos_suspend_prepare(void)
 {
+	int ret;
+
+	/*
+	 * REVISIT: It would be better if struct platform_suspend_ops
+	 * .prepare handler get the suspend_state_t as a parameter to
+	 * avoid hard-coding the suspend to mem state. It's safe to do
+	 * it now only because the suspend_valid_only_mem function is
+	 * used as the .valid callback used to check if a given state
+	 * is supported by the platform anyways.
+	 */
+	ret = regulator_suspend_prepare(PM_SUSPEND_MEM);
+	if (ret) {
+		pr_err("Failed to prepare regulators for suspend (%d)\n", ret);
+		return ret;
+	}
+
 	s3c_pm_check_prepare();
 
 	return 0;
@@ -450,7 +467,13 @@ static int exynos_suspend_prepare(void)
 
 static void exynos_suspend_finish(void)
 {
+	int ret;
+
 	s3c_pm_check_cleanup();
+
+	ret = regulator_suspend_finish();
+	if (ret)
+		pr_warn("Failed to resume regulators from suspend (%d)\n", ret);
 }
 
 static const struct platform_suspend_ops exynos_suspend_ops = {
-- 
2.1.0

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Doug Anderson]

Javier,

On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> The regulator framework has a set of helpers functions to be used when
> the system is entering and leaving from suspend but these are not called
> on Exynos platforms. This means that the .set_suspend_* function handlers
> defined by regulator drivers are not called when the system is suspended.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Doug Anderson <dianders@chromium.org>

I think you forgot to carry Chanwoo's review:

Reviewed-by: Chanwoo Choi<cw00.choi@samsung.com>

I don't think you've made any changes that would need to invalidate his review.

Re: [PATCH v3 1/2] Revert "mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption"

[Doug Anderson]

Javier,

On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> This reverts commit b7cde7078d2344073c310aa65fc2b0a845d2cb5b
> ("mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption")
>
> Commit b7cde7078d23 called regulator_suspend_prepare() to prepare the
> regulators for a suspend state. But it did from the device pm suspend
> handler while the regulator suspend prepare function iterates over all
> regulators and not only the one managed by this device so it doesn't
> seems to be correct to call it from within a device driver.
>
> It is better to call the regulator suspend prepare/finish functions
> from platform code instead so this patch reverts the mentioned commit.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Reviewed-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Javier Martinez Canillas]

Hello Doug,

On 10/20/2014 11:40 PM, Doug Anderson wrote:
> Javier,
> 
> On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> The regulator framework has a set of helpers functions to be used when
>> the system is entering and leaving from suspend but these are not called
>> on Exynos platforms. This means that the .set_suspend_* function handlers
>> defined by regulator drivers are not called when the system is suspended.
>>
>> Suggested-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
> 
> I think you forgot to carry Chanwoo's review:
> 
> Reviewed-by: Chanwoo Choi<cw00.choi@samsung.com>
> 
> I don't think you've made any changes that would need to invalidate his review.
> 

Yes, I forgot to add the tag to my local branch so it was missed when generating
the patch for the new version, sorry about that...

Best regards,
Javier

Re: [PATCH v3 1/2] Revert "mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption"

[Chanwoo Choi]

Hi Javier,

On 10/21/2014 06:05 AM, Javier Martinez Canillas wrote:
> This reverts commit b7cde7078d2344073c310aa65fc2b0a845d2cb5b
> ("mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption")
> 
> Commit b7cde7078d23 called regulator_suspend_prepare() to prepare the
> regulators for a suspend state. But it did from the device pm suspend
> handler while the regulator suspend prepare function iterates over all
> regulators and not only the one managed by this device so it doesn't
> seems to be correct to call it from within a device driver.
> 
> It is better to call the regulator suspend prepare/finish functions
> from platform code instead so this patch reverts the mentioned commit.
> 
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/Kconfig    |  1 -
>  drivers/mfd/sec-core.c | 10 ----------
>  2 files changed, 11 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1456ea7..fd8cc4c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -655,7 +655,6 @@ config MFD_SEC_CORE
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> -	select REGULATOR
>  	help
>  	 Support for the Samsung Electronics MFD series.
>  	 This driver provides common support for accessing the device,
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index dba7e2b..5993608 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -31,7 +31,6 @@
>  #include <linux/mfd/samsung/s2mpu02.h>
>  #include <linux/mfd/samsung/s5m8763.h>
>  #include <linux/mfd/samsung/s5m8767.h>
> -#include <linux/regulator/machine.h>
>  #include <linux/regmap.h>
>  
>  static const struct mfd_cell s5m8751_devs[] = {
> @@ -432,15 +431,6 @@ static int sec_pmic_suspend(struct device *dev)
>  	 */
>  	disable_irq(sec_pmic->irq);
>  
> -	switch (sec_pmic->device_type) {
> -	case S2MPS14X:
> -	case S2MPU02:
> -		regulator_suspend_prepare(PM_SUSPEND_MEM);
> -		break;
> -	default:
> -		break;
> -	}
> -
>  	return 0;
>  }
>  
> 

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi

Re: [PATCH v3 0/2] ARM: EXYNOS: Call regulator suspend prepare/finish

[Krzysztof Kozłowski]

On 20.10.2014 23:05, Javier Martinez Canillas wrote:
> Hello,
> 
> This series add support for Exynos platforms to prepare regulators for
> system suspend. The regulator core has a set of helpers functions to be
> used when the system is entering and leaving from a suspend state but
> currently there is only one user in mainline.
> 
> This user is drivers/mfd/sec-core.c but it calls regulator_suspend_prepare()
> from within the driver power-management suspend function. This does not
> seems to be correct since the regulator suspend prepare function affects all
> regulators in the system and not only the ones managed by this device.
> 
> So patch #1 in this series revert the commit that introduced that change and
> patch #2 calls the regulator framework suspend/finish functions from the
> Exynos platform power-management code. The first patch should be queued through
> the mfd tree and the second through the linux-samsung tree.
> 
> Changes since v2:
>  - Check for regulator_suspend_finish() return value for an error.
>    Suggested by Doug Anderson.
> 
> Changes since v1:
>  - Remove the call to regulator_suspend_prepare() from drivers/mfd/sec-core.c
>    as suggested by Doug Anderson.
>  - Call regulator_suspend_prepare() before s3c_pm_check_prepare() as suggested
>    by Doug Anderson.
>  - Added Lee Jones to cc list since there is a change for the mfd framework.
> 
> Javier Martinez Canillas (2):
>   Revert "mfd: sec-core: Prepare regulators for suspend state to reduce
>     power-consumption"
>   ARM: EXYNOS: Call regulator core suspend prepare and finish functions
> 
>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>  drivers/mfd/Kconfig            |  1 -
>  drivers/mfd/sec-core.c         | 10 ----------
>  3 files changed, 23 insertions(+), 11 deletions(-)

Patchset tested on Trats2 board (Exynos4412 with max77686 PMIC, modified
DTS to disable buck[134] during suspend to RAM).

Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v3 1/2] Revert "mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption"

[Lee Jones]

On Mon, 20 Oct 2014, Javier Martinez Canillas wrote:

> This reverts commit b7cde7078d2344073c310aa65fc2b0a845d2cb5b
> ("mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption")
> 
> Commit b7cde7078d23 called regulator_suspend_prepare() to prepare the
> regulators for a suspend state. But it did from the device pm suspend
> handler while the regulator suspend prepare function iterates over all
> regulators and not only the one managed by this device so it doesn't
> seems to be correct to call it from within a device driver.
> 
> It is better to call the regulator suspend prepare/finish functions
> from platform code instead so this patch reverts the mentioned commit.
> 
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/Kconfig    |  1 -
>  drivers/mfd/sec-core.c | 10 ----------
>  2 files changed, 11 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1456ea7..fd8cc4c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -655,7 +655,6 @@ config MFD_SEC_CORE
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> -	select REGULATOR
>  	help
>  	 Support for the Samsung Electronics MFD series.
>  	 This driver provides common support for accessing the device,
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index dba7e2b..5993608 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -31,7 +31,6 @@
>  #include <linux/mfd/samsung/s2mpu02.h>
>  #include <linux/mfd/samsung/s5m8763.h>
>  #include <linux/mfd/samsung/s5m8767.h>
> -#include <linux/regulator/machine.h>
>  #include <linux/regmap.h>
>  
>  static const struct mfd_cell s5m8751_devs[] = {
> @@ -432,15 +431,6 @@ static int sec_pmic_suspend(struct device *dev)
>  	 */
>  	disable_irq(sec_pmic->irq);
>  
> -	switch (sec_pmic->device_type) {
> -	case S2MPS14X:
> -	case S2MPU02:
> -		regulator_suspend_prepare(PM_SUSPEND_MEM);
> -		break;
> -	default:
> -		break;
> -	}
> -
>  	return 0;
>  }
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Javier Martinez Canillas]

Hello Kukjin,

On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> The regulator framework has a set of helpers functions to be used when
> the system is entering and leaving from suspend but these are not called
> on Exynos platforms. This means that the .set_suspend_* function handlers
> defined by regulator drivers are not called when the system is suspended.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> ---
>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>

Any comments on this patch?

Best regards,
Javier

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Javier Martinez Canillas]

Hello Kukjin,

On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Kukjin,
>
> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> The regulator framework has a set of helpers functions to be used when
>> the system is entering and leaving from suspend but these are not called
>> on Exynos platforms. This means that the .set_suspend_* function handlers
>> defined by regulator drivers are not called when the system is suspended.
>>
>> Suggested-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>> ---
>>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>
> Any comments on this patch?
>

Just a gentle reminder about this patch.

Best regards,
Javier

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Olof Johansson]

On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Kukjin,
>
> On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>> Hello Kukjin,
>>
>> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
>> <javier.martinez@collabora.co.uk> wrote:
>>> The regulator framework has a set of helpers functions to be used when
>>> the system is entering and leaving from suspend but these are not called
>>> on Exynos platforms. This means that the .set_suspend_* function handlers
>>> defined by regulator drivers are not called when the system is suspended.
>>>
>>> Suggested-by: Doug Anderson <dianders@chromium.org>
>>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>> ---
>>>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>
>>
>> Any comments on this patch?
>>
>
> Just a gentle reminder about this patch.

Kukjin, should I just apply this directly since you seem to be busy?


Thanks,

-Olof

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Kukjin Kim]

On 11/13/14 02:02, Olof Johansson wrote:
> On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas
> <javier@dowhile0.org>  wrote:
>> Hello Kukjin,
>>
>> On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
>> <javier@dowhile0.org>  wrote:
>>> Hello Kukjin,
>>>
>>> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
>>> <javier.martinez@collabora.co.uk>  wrote:
>>>> The regulator framework has a set of helpers functions to be used when
>>>> the system is entering and leaving from suspend but these are not called
>>>> on Exynos platforms. This means that the .set_suspend_* function handlers
>>>> defined by regulator drivers are not called when the system is suspended.
>>>>
>>>> Suggested-by: Doug Anderson<dianders@chromium.org>
>>>> Signed-off-by: Javier Martinez Canillas<javier.martinez@collabora.co.uk>
>>>> Reviewed-by: Doug Anderson<dianders@chromium.org>
>>>> ---
>>>>   arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>>>   1 file changed, 23 insertions(+)
>>>>
>>>
>>> Any comments on this patch?
>>>
>>
>> Just a gentle reminder about this patch.
>
> Kukjin, should I just apply this directly since you seem to be busy?
>
Oh, sorry. I'm here and applying this patch into samsung tree.

Thanks,
Kukjin

Re: [PATCH v3 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

[Javier Martinez Canillas]

Hello Kukjin,

On 11/13/2014 03:12 AM, Kukjin Kim wrote:
>>
> Oh, sorry. I'm here and applying this patch into samsung tree.
> 

Great, thanks a lot!

Best regards,
Javier