[PATCH 1/3] PM: Provide an always on power domain governor

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

Since systems are likely to have power domains that can't be turned off
for various reasons at least temporarily while implementing power domain
support provide a default governor which will always refuse to power off
the domain, saving platforms having to implement their own.

Since the code is so tiny don't bother with a Kconfig symbol for it.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/base/power/domain_governor.c |   13 +++++++++++++
 include/linux/pm_domain.h            |    2 ++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index da78540..51527ee 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -141,3 +141,16 @@ struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
 	.power_down_ok = default_power_down_ok,
 };
+
+static bool always_on_power_down_ok(struct dev_pm_domain *domain)
+{
+	return false;
+}
+
+/**
+ * pm_genpd_gov_always_on - A governor implementing an always-on policy
+ */
+struct dev_power_governor pm_domain_always_on_gov = {
+	.power_down_ok = always_on_power_down_ok,
+	.stop_ok = default_stop_ok,
+};
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index fbb81bc..fda14f1 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -139,6 +139,7 @@ extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
 
 extern bool default_stop_ok(struct device *dev);
 
+extern struct dev_power_governor pm_domain_always_on_gov;
 #else
 
 static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
@@ -192,6 +193,7 @@ static inline bool default_stop_ok(struct device *dev)
 {
 	return false;
 }
+#define pm_domain_always_on_gov NULL
 #endif
 
 static inline int pm_genpd_remove_callbacks(struct device *dev)
-- 
1.7.7.3

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Mark Brown]

Saves a tiny amount of code.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   43 +----------------------------------
 1 files changed, 2 insertions(+), 41 deletions(-)

diff --git a/arch/arm/mach-shmobile/pm-sh7372.c b/arch/arm/mach-shmobile/pm-sh7372.c
index adf1765..5f7d575 100644
--- a/arch/arm/mach-shmobile/pm-sh7372.c
+++ b/arch/arm/mach-shmobile/pm-sh7372.c
@@ -162,45 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
 	return active_wakeup ? active_wakeup(dev) : true;
 }
 
-static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
-{
-	return false;
-}
-
-struct dev_power_governor sh7372_always_on_gov = {
-	.power_down_ok = sh7372_power_down_forbidden,
-	.stop_ok = default_stop_ok,
-};
-
-static int sh7372_stop_dev(struct device *dev)
-{
-	int (*stop)(struct device *dev);
-
-	stop = dev_gpd_data(dev)->ops.stop;
-	if (stop) {
-		int ret = stop(dev);
-		if (ret)
-			return ret;
-	}
-	return pm_clk_suspend(dev);
-}
-
-static int sh7372_start_dev(struct device *dev)
-{
-	int (*start)(struct device *dev);
-	int ret;
-
-	ret = pm_clk_resume(dev);
-	if (ret)
-		return ret;
-
-	start = dev_gpd_data(dev)->ops.start;
-	if (start)
-		ret = start(dev);
-
-	return ret;
-}
-
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
@@ -246,7 +207,7 @@ struct sh7372_pm_domain sh7372_d4 = {
 
 struct sh7372_pm_domain sh7372_a4r = {
 	.bit_shift = 5,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 	.stay_on = true,
@@ -262,7 +223,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
 
 struct sh7372_pm_domain sh7372_a3sp = {
 	.bit_shift = 11,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
 };
 
-- 
1.7.7.3

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Rafael J. Wysocki]

Hi,

On Thursday, December 01, 2011, Mark Brown wrote:
> Saves a tiny amount of code.

How so?

> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  arch/arm/mach-shmobile/pm-sh7372.c |   43 +----------------------------------
>  1 files changed, 2 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/pm-sh7372.c b/arch/arm/mach-shmobile/pm-sh7372.c
> index adf1765..5f7d575 100644
> --- a/arch/arm/mach-shmobile/pm-sh7372.c
> +++ b/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -162,45 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
>  	return active_wakeup ? active_wakeup(dev) : true;
>  }
>  
> -static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
> -{
> -	return false;
> -}
> -
> -struct dev_power_governor sh7372_always_on_gov = {
> -	.power_down_ok = sh7372_power_down_forbidden,
> -	.stop_ok = default_stop_ok,
> -};
> -
> -static int sh7372_stop_dev(struct device *dev)
> -{
> -	int (*stop)(struct device *dev);
> -
> -	stop = dev_gpd_data(dev)->ops.stop;
> -	if (stop) {
> -		int ret = stop(dev);
> -		if (ret)
> -			return ret;
> -	}
> -	return pm_clk_suspend(dev);
> -}
> -
> -static int sh7372_start_dev(struct device *dev)
> -{
> -	int (*start)(struct device *dev);
> -	int ret;
> -
> -	ret = pm_clk_resume(dev);
> -	if (ret)
> -		return ret;
> -
> -	start = dev_gpd_data(dev)->ops.start;
> -	if (start)
> -		ret = start(dev);
> -
> -	return ret;
> -}
> -

The above functions are actually necessary, please don't remove them
without introducing replacements.

>  void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
>  {
>  	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
> @@ -246,7 +207,7 @@ struct sh7372_pm_domain sh7372_d4 = {
>  
>  struct sh7372_pm_domain sh7372_a4r = {
>  	.bit_shift = 5,
> -	.gov = &sh7372_always_on_gov,
> +	.gov = &pm_domain_always_on_gov,
>  	.suspend = sh7372_a4r_suspend,
>  	.resume = sh7372_intcs_resume,
>  	.stay_on = true,
> @@ -262,7 +223,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
>  
>  struct sh7372_pm_domain sh7372_a3sp = {
>  	.bit_shift = 11,
> -	.gov = &sh7372_always_on_gov,
> +	.gov = &pm_domain_always_on_gov,
>  	.no_debug = true,
>  };

Thanks,
Rafael

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Mark Brown]

On Thu, Dec 01, 2011 at 09:37:07PM +0100, Rafael J. Wysocki wrote:
> On Thursday, December 01, 2011, Mark Brown wrote:
> > Saves a tiny amount of code.

> How so?

It saves source code size when we get the second user, by itself you're
right it doesn't achieve anything (except avoiding redundancy once 1/3
gets applied I guess).

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Rafael J. Wysocki]

On Friday, December 02, 2011, Mark Brown wrote:
> On Thu, Dec 01, 2011 at 09:37:07PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, December 01, 2011, Mark Brown wrote:
> > > Saves a tiny amount of code.
> 
> > How so?
> 
> It saves source code size when we get the second user, by itself you're
> right it doesn't achieve anything (except avoiding redundancy once 1/3
> gets applied I guess).

But the patch removed useful code without adding any replacements, as far
as I could see it.

Thanks,
Rafael

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Mark Brown]

On Fri, Dec 02, 2011 at 11:36:19PM +0100, Rafael J. Wysocki wrote:
> On Friday, December 02, 2011, Mark Brown wrote:

> > It saves source code size when we get the second user, by itself you're
> > right it doesn't achieve anything (except avoiding redundancy once 1/3
> > gets applied I guess).

> But the patch removed useful code without adding any replacements, as far
> as I could see it.

The first patch in the series added the replacement as generic code,
this patch is deleting the shmobile specific implementation and using
the generic code instead.

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Rafael J. Wysocki]

On Saturday, December 03, 2011, Mark Brown wrote:
> On Fri, Dec 02, 2011 at 11:36:19PM +0100, Rafael J. Wysocki wrote:
> > On Friday, December 02, 2011, Mark Brown wrote:
> 
> > > It saves source code size when we get the second user, by itself you're
> > > right it doesn't achieve anything (except avoiding redundancy once 1/3
> > > gets applied I guess).
> 
> > But the patch removed useful code without adding any replacements, as far
> > as I could see it.
> 
> The first patch in the series added the replacement as generic code,
> this patch is deleting the shmobile specific implementation and using
> the generic code instead.

No, the first patch didn't add any replacements for the functions that the
second patch removed.  At least I don't see the replacements in the first
patch.

Thanks,
Rafael

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Mark Brown]

On Sat, Dec 03, 2011 at 11:31:07PM +0100, Rafael J. Wysocki wrote:
> On Saturday, December 03, 2011, Mark Brown wrote:

> > The first patch in the series added the replacement as generic code,
> > this patch is deleting the shmobile specific implementation and using
> > the generic code instead.

> No, the first patch didn't add any replacements for the functions that the
> second patch removed.  At least I don't see the replacements in the first
> patch.

Oh, right.  It'd really help if you were more specific.  I'd probably
have realised earlier that you are probably talking about the stop_dev
and start_dev functions which appear to have got caught up in the mess
of a rebase somehow - I didn't intentionally remove them as their
absence from the patch description and the lack of any removal of
references to them should suggest.

[PATCH 2/3] ARM: mach-shmobile: Use common always on power domain governor

[Rafael J. Wysocki]

On Sunday, December 04, 2011, Mark Brown wrote:
> On Sat, Dec 03, 2011 at 11:31:07PM +0100, Rafael J. Wysocki wrote:
> > On Saturday, December 03, 2011, Mark Brown wrote:
> 
> > > The first patch in the series added the replacement as generic code,
> > > this patch is deleting the shmobile specific implementation and using
> > > the generic code instead.
> 
> > No, the first patch didn't add any replacements for the functions that the
> > second patch removed.  At least I don't see the replacements in the first
> > patch.
> 
> Oh, right.  It'd really help if you were more specific.  I'd probably
> have realised earlier that you are probably talking about the stop_dev
> and start_dev functions which appear to have got caught up in the mess
> of a rebase somehow - I didn't intentionally remove them as their
> absence from the patch description and the lack of any removal of
> references to them should suggest.

Well, I mentioned these two functions specifically in my first reply to the
patch message. :-)

Thanks,
Rafael

[PATCH v2] ARM: mach-shmobile: Use common always on power domain governor

[Mark Brown]

Saves a tiny amount of code.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-shmobile/pm-sh7372.c b/arch/arm/mach-shmobile/pm-sh7372.c
index adf1765..7d7cdfc 100644
--- a/arch/arm/mach-shmobile/pm-sh7372.c
+++ b/arch/arm/mach-shmobile/pm-sh7372.c
@@ -162,16 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
 	return active_wakeup ? active_wakeup(dev) : true;
 }
 
-static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
-{
-	return false;
-}
-
-struct dev_power_governor sh7372_always_on_gov = {
-	.power_down_ok = sh7372_power_down_forbidden,
-	.stop_ok = default_stop_ok,
-};
-
 static int sh7372_stop_dev(struct device *dev)
 {
 	int (*stop)(struct device *dev);
@@ -246,7 +236,7 @@ struct sh7372_pm_domain sh7372_d4 = {
 
 struct sh7372_pm_domain sh7372_a4r = {
 	.bit_shift = 5,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 	.stay_on = true,
@@ -262,7 +252,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
 
 struct sh7372_pm_domain sh7372_a3sp = {
 	.bit_shift = 11,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
 };
 
-- 
1.7.7.3

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Mark Brown]

The S3C64xx SoCs contain a set of gateable power domains which can be
enabled and disabled at runtime in order to save power.  Use the generic
power domain code to implement support for these in software, enabling
runtime control of most domains:

 - ETM (not supported in mainline).
 - Domain G: 3D acceleration (no mainline support).
 - Domain V: MFC (no mainline support).
 - Domain I: JPEG and camera interface (no mainline support).
 - Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
 - Domain S: Security (no mainline support).
 - Domain F: LCD (driver already uses runtime PM), post processing and
   rotation (no mainline support).

The IROM domain is marked as always enabled as we should arrange for it
to be enabled when we suspend which will need a bit more work.

Due to all the conditional device registration that the platform does
wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device
into the power domain after the machines have registered, looking for
platform data to tell if the device was registered. Since currently only
Cragganmore actually sets up PM that is the only machine updated.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---

Kikjin, this superceeds my previous patch unconditionally disabling some
of the domains.  I've given it quite a bit of testing and it appears to
be working well on my systems though I'm not sure the power saving is
really all that much to write home about.  

 arch/arm/mach-s3c64xx/Kconfig           |    1 +
 arch/arm/mach-s3c64xx/mach-crag6410.c   |    2 +-
 arch/arm/mach-s3c64xx/pm.c              |  173 ++++++++++++++++++++++++++++++-
 arch/arm/plat-samsung/include/plat/pm.h |    6 +
 4 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig
index 4d8c489..5c6c22e 100644
--- a/arch/arm/mach-s3c64xx/Kconfig
+++ b/arch/arm/mach-s3c64xx/Kconfig
@@ -8,6 +8,7 @@ config PLAT_S3C64XX
 	bool
 	depends on ARCH_S3C64XX
 	select SAMSUNG_WAKEMASK
+	select PM_GENERIC_DOMAINS
 	default y
 	help
 	  Base platform code for any Samsung S3C64XX device
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
index 98d42b3..4ce4a74 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -743,7 +743,7 @@ static void __init crag6410_machine_init(void)
 
 	regulator_has_full_constraints();
 
-	s3c_pm_init();
+	s3c64xx_pm_init();
 }
 
 MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
index b375cd5..aa7b5d1 100644
--- a/arch/arm/mach-s3c64xx/pm.c
+++ b/arch/arm/mach-s3c64xx/pm.c
@@ -17,10 +17,12 @@
 #include <linux/serial_core.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/pm_domain.h>
 
 #include <mach/map.h>
 #include <mach/irqs.h>
 
+#include <plat/devs.h>
 #include <plat/pm.h>
 #include <plat/wakeup-mask.h>
 
@@ -31,6 +33,145 @@
 #include <mach/regs-gpio-memport.h>
 #include <mach/regs-modem.h>
 
+struct s3c64xx_pm_domain {
+	char *const name;
+	u32 ena;
+	u32 pwr_stat;
+	struct generic_pm_domain pd;
+};
+
+static int s3c64xx_pd_off(struct generic_pm_domain *domain)
+{
+	struct s3c64xx_pm_domain *pd;
+	u32 val;
+
+	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
+
+	val = __raw_readl(S3C64XX_NORMAL_CFG);
+	val &= ~(pd->ena);
+	__raw_writel(val, S3C64XX_NORMAL_CFG);
+
+	return 0;
+}
+
+static int s3c64xx_pd_on(struct generic_pm_domain *domain)
+{
+	struct s3c64xx_pm_domain *pd;
+	u32 val;
+	long retry = 1000000L;
+
+	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
+
+	val = __raw_readl(S3C64XX_NORMAL_CFG);
+	val |= pd->ena;
+	__raw_writel(val, S3C64XX_NORMAL_CFG);
+
+	/* Not all domains provide power status readback */
+	if (pd->pwr_stat) {
+		while (retry--)
+			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
+				break;
+		if (!retry) {
+			pr_err("Failed to start domain %s\n", pd->name);
+			return -EBUSY;
+		}
+	}
+
+	return 0;
+}
+
+static struct s3c64xx_pm_domain s3c64xx_pm_irom = {
+	.name = "IROM",
+	.ena = S3C64XX_NORMALCFG_IROM_ON,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_etm = {
+	.name = "ETM",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_ETM_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_ETM,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_s = {
+	.name = "S",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_S_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_S,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_f = {
+	.name = "F",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_F_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_F,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_p = {
+	.name = "P",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_P_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_P,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_i = {
+	.name = "I",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_I_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_I,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_g = {
+	.name = "G",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_G_ON,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_v = {
+	.name = "V",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_V_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_V,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain *s3c64xx_always_on_pm_domains[] = {
+	&s3c64xx_pm_irom,
+};
+
+static struct s3c64xx_pm_domain *s3c64xx_pm_domains[] = {
+	&s3c64xx_pm_etm,
+	&s3c64xx_pm_g,
+	&s3c64xx_pm_v,
+	&s3c64xx_pm_i,
+	&s3c64xx_pm_p,
+	&s3c64xx_pm_s,
+	&s3c64xx_pm_f,
+};
+
 #ifdef CONFIG_S3C_PM_DEBUG_LED_SMDK
 void s3c_pm_debug_smdkled(u32 set, u32 clear)
 {
@@ -89,6 +230,8 @@ static struct sleep_save misc_save[] = {
 
 	SAVE_ITEM(S3C64XX_SDMA_SEL),
 	SAVE_ITEM(S3C64XX_MODEM_MIFPCON),
+
+	SAVE_ITEM(S3C64XX_NORMAL_CFG),
 };
 
 void s3c_pm_configure_extint(void)
@@ -179,7 +322,26 @@ static void s3c64xx_pm_prepare(void)
 	__raw_writel(__raw_readl(S3C64XX_WAKEUP_STAT), S3C64XX_WAKEUP_STAT);
 }
 
-static int s3c64xx_pm_init(void)
+int __init s3c64xx_pm_init(void)
+{
+	int i;
+
+	s3c_pm_init();
+
+	for (i = 0; i < ARRAY_SIZE(s3c64xx_always_on_pm_domains); i++)
+		pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
+			      &pm_domain_always_on_gov, false);
+
+	for (i = 0; i < ARRAY_SIZE(s3c64xx_pm_domains); i++)
+		pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
+
+	if (dev_get_platdata(&s3c_device_fb.dev))
+		pm_genpd_add_device(&s3c64xx_pm_f.pd, &s3c_device_fb.dev);
+
+	return 0;
+}
+
+static __init int s3c64xx_pm_initcall(void)
 {
 	pm_cpu_prep = s3c64xx_pm_prepare;
 	pm_cpu_sleep = s3c64xx_cpu_suspend;
@@ -198,5 +360,12 @@ static int s3c64xx_pm_init(void)
 
 	return 0;
 }
+arch_initcall(s3c64xx_pm_initcall);
+
+static __init int s3c64xx_pm_late_initcall(void)
+{
+	pm_genpd_poweroff_unused();
 
-arch_initcall(s3c64xx_pm_init);
+	return 0;
+}
+late_initcall(s3c64xx_pm_late_initcall);
diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-samsung/include/plat/pm.h
index dcf6870..a6bdee2 100644
--- a/arch/arm/plat-samsung/include/plat/pm.h
+++ b/arch/arm/plat-samsung/include/plat/pm.h
@@ -22,6 +22,7 @@ struct sys_device;
 #ifdef CONFIG_PM
 
 extern __init int s3c_pm_init(void);
+extern __init int s3c64xx_pm_init(void);
 
 #else
 
@@ -29,6 +30,11 @@ static inline int s3c_pm_init(void)
 {
 	return 0;
 }
+
+static inline int s3c64xx_pm_init(void)
+{
+	return 0;
+}
 #endif
 
 /* configuration for the IRQ mask over sleep */
-- 
1.7.7.3

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Kyungmin Park]

Hi Mark,

I'm not sure what's the next step at s3c64xx for generic power domain.
Related with exysno4 series, it's helpful to read following threads.
http://68.183.106.108/lists/linux-pm/msg26291.html

"I don't think we should control/gate the clocks with regarding power
domain" from Mr. Kim

Thank you,
Kyungmin Park

On 12/2/11, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> The S3C64xx SoCs contain a set of gateable power domains which can be
> enabled and disabled at runtime in order to save power.  Use the generic
> power domain code to implement support for these in software, enabling
> runtime control of most domains:
>
>  - ETM (not supported in mainline).
>  - Domain G: 3D acceleration (no mainline support).
>  - Domain V: MFC (no mainline support).
>  - Domain I: JPEG and camera interface (no mainline support).
>  - Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
>  - Domain S: Security (no mainline support).
>  - Domain F: LCD (driver already uses runtime PM), post processing and
>    rotation (no mainline support).
>
> The IROM domain is marked as always enabled as we should arrange for it
> to be enabled when we suspend which will need a bit more work.
>
> Due to all the conditional device registration that the platform does
> wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device
> into the power domain after the machines have registered, looking for
> platform data to tell if the device was registered. Since currently only
> Cragganmore actually sets up PM that is the only machine updated.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>
> Kikjin, this superceeds my previous patch unconditionally disabling some
> of the domains.  I've given it quite a bit of testing and it appears to
> be working well on my systems though I'm not sure the power saving is
> really all that much to write home about.
>
>  arch/arm/mach-s3c64xx/Kconfig           |    1 +
>  arch/arm/mach-s3c64xx/mach-crag6410.c   |    2 +-
>  arch/arm/mach-s3c64xx/pm.c              |  173
> ++++++++++++++++++++++++++++++-
>  arch/arm/plat-samsung/include/plat/pm.h |    6 +
>  4 files changed, 179 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig
> index 4d8c489..5c6c22e 100644
> --- a/arch/arm/mach-s3c64xx/Kconfig
> +++ b/arch/arm/mach-s3c64xx/Kconfig
> @@ -8,6 +8,7 @@ config PLAT_S3C64XX
>  	bool
>  	depends on ARCH_S3C64XX
>  	select SAMSUNG_WAKEMASK
> +	select PM_GENERIC_DOMAINS
>  	default y
>  	help
>  	  Base platform code for any Samsung S3C64XX device
> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c
> b/arch/arm/mach-s3c64xx/mach-crag6410.c
> index 98d42b3..4ce4a74 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> @@ -743,7 +743,7 @@ static void __init crag6410_machine_init(void)
>
>  	regulator_has_full_constraints();
>
> -	s3c_pm_init();
> +	s3c64xx_pm_init();
>  }
>
>  MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
> diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
> index b375cd5..aa7b5d1 100644
> --- a/arch/arm/mach-s3c64xx/pm.c
> +++ b/arch/arm/mach-s3c64xx/pm.c
> @@ -17,10 +17,12 @@
>  #include <linux/serial_core.h>
>  #include <linux/io.h>
>  #include <linux/gpio.h>
> +#include <linux/pm_domain.h>
>
>  #include <mach/map.h>
>  #include <mach/irqs.h>
>
> +#include <plat/devs.h>
>  #include <plat/pm.h>
>  #include <plat/wakeup-mask.h>
>
> @@ -31,6 +33,145 @@
>  #include <mach/regs-gpio-memport.h>
>  #include <mach/regs-modem.h>
>
> +struct s3c64xx_pm_domain {
> +	char *const name;
> +	u32 ena;
> +	u32 pwr_stat;
> +	struct generic_pm_domain pd;
> +};
> +
> +static int s3c64xx_pd_off(struct generic_pm_domain *domain)
> +{
> +	struct s3c64xx_pm_domain *pd;
> +	u32 val;
> +
> +	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
> +
> +	val = __raw_readl(S3C64XX_NORMAL_CFG);
> +	val &= ~(pd->ena);
> +	__raw_writel(val, S3C64XX_NORMAL_CFG);
> +
> +	return 0;
> +}
> +
> +static int s3c64xx_pd_on(struct generic_pm_domain *domain)
> +{
> +	struct s3c64xx_pm_domain *pd;
> +	u32 val;
> +	long retry = 1000000L;
> +
> +	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
> +
> +	val = __raw_readl(S3C64XX_NORMAL_CFG);
> +	val |= pd->ena;
> +	__raw_writel(val, S3C64XX_NORMAL_CFG);
> +
> +	/* Not all domains provide power status readback */
> +	if (pd->pwr_stat) {
> +		while (retry--)
> +			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
> +				break;
> +		if (!retry) {
> +			pr_err("Failed to start domain %s\n", pd->name);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_irom = {
> +	.name = "IROM",
> +	.ena = S3C64XX_NORMALCFG_IROM_ON,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_etm = {
> +	.name = "ETM",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_ETM_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_ETM,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_s = {
> +	.name = "S",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_S_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_S,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_f = {
> +	.name = "F",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_F_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_F,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_p = {
> +	.name = "P",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_P_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_P,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_i = {
> +	.name = "I",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_I_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_I,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_g = {
> +	.name = "G",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_G_ON,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain s3c64xx_pm_v = {
> +	.name = "V",
> +	.ena = S3C64XX_NORMALCFG_DOMAIN_V_ON,
> +	.pwr_stat = S3C64XX_BLKPWRSTAT_V,
> +	.pd = {
> +		.power_off = s3c64xx_pd_off,
> +		.power_on = s3c64xx_pd_on,
> +	},
> +};
> +
> +static struct s3c64xx_pm_domain *s3c64xx_always_on_pm_domains[] = {
> +	&s3c64xx_pm_irom,
> +};
> +
> +static struct s3c64xx_pm_domain *s3c64xx_pm_domains[] = {
> +	&s3c64xx_pm_etm,
> +	&s3c64xx_pm_g,
> +	&s3c64xx_pm_v,
> +	&s3c64xx_pm_i,
> +	&s3c64xx_pm_p,
> +	&s3c64xx_pm_s,
> +	&s3c64xx_pm_f,
> +};
> +
>  #ifdef CONFIG_S3C_PM_DEBUG_LED_SMDK
>  void s3c_pm_debug_smdkled(u32 set, u32 clear)
>  {
> @@ -89,6 +230,8 @@ static struct sleep_save misc_save[] = {
>
>  	SAVE_ITEM(S3C64XX_SDMA_SEL),
>  	SAVE_ITEM(S3C64XX_MODEM_MIFPCON),
> +
> +	SAVE_ITEM(S3C64XX_NORMAL_CFG),
>  };
>
>  void s3c_pm_configure_extint(void)
> @@ -179,7 +322,26 @@ static void s3c64xx_pm_prepare(void)
>  	__raw_writel(__raw_readl(S3C64XX_WAKEUP_STAT), S3C64XX_WAKEUP_STAT);
>  }
>
> -static int s3c64xx_pm_init(void)
> +int __init s3c64xx_pm_init(void)
> +{
> +	int i;
> +
> +	s3c_pm_init();
> +
> +	for (i = 0; i < ARRAY_SIZE(s3c64xx_always_on_pm_domains); i++)
> +		pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
> +			      &pm_domain_always_on_gov, false);
> +
> +	for (i = 0; i < ARRAY_SIZE(s3c64xx_pm_domains); i++)
> +		pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
> +
> +	if (dev_get_platdata(&s3c_device_fb.dev))
> +		pm_genpd_add_device(&s3c64xx_pm_f.pd, &s3c_device_fb.dev);
> +
> +	return 0;
> +}
> +
> +static __init int s3c64xx_pm_initcall(void)
>  {
>  	pm_cpu_prep = s3c64xx_pm_prepare;
>  	pm_cpu_sleep = s3c64xx_cpu_suspend;
> @@ -198,5 +360,12 @@ static int s3c64xx_pm_init(void)
>
>  	return 0;
>  }
> +arch_initcall(s3c64xx_pm_initcall);
> +
> +static __init int s3c64xx_pm_late_initcall(void)
> +{
> +	pm_genpd_poweroff_unused();
>
> -arch_initcall(s3c64xx_pm_init);
> +	return 0;
> +}
> +late_initcall(s3c64xx_pm_late_initcall);
> diff --git a/arch/arm/plat-samsung/include/plat/pm.h
> b/arch/arm/plat-samsung/include/plat/pm.h
> index dcf6870..a6bdee2 100644
> --- a/arch/arm/plat-samsung/include/plat/pm.h
> +++ b/arch/arm/plat-samsung/include/plat/pm.h
> @@ -22,6 +22,7 @@ struct sys_device;
>  #ifdef CONFIG_PM
>
>  extern __init int s3c_pm_init(void);
> +extern __init int s3c64xx_pm_init(void);
>
>  #else
>
> @@ -29,6 +30,11 @@ static inline int s3c_pm_init(void)
>  {
>  	return 0;
>  }
> +
> +static inline int s3c64xx_pm_init(void)
> +{
> +	return 0;
> +}
>  #endif
>
>  /* configuration for the IRQ mask over sleep */
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Mark Brown]

On Fri, Dec 02, 2011 at 09:35:44AM +0900, Kyungmin Park wrote:

> I'm not sure what's the next step at s3c64xx for generic power domain.
> Related with exysno4 series, it's helpful to read following threads.
> http://68.183.106.108/lists/linux-pm/msg26291.html

> "I don't think we should control/gate the clocks with regarding power
> domain" from Mr. Kim

I tend to agree with him that gating the clocks automatically using
runtime PM would be nice as if nothing else it saves code in drivers.
I'm not sure how easy it's going to be to transition to doing that in
one jump, though - there's a lot of SoCs sharing IP right back to
S3C24xx.

Of course it should be fairly easy for most of the domains on s3c64xx
right now as there's only one actual driver involved in mainline, we
should just be gating almost all of the clocks concerned immediately
after we boot (assuming they aren't gated by default, I'd need to check)
and never touching them again anyway.  It'd only be the framebuffer
clocks that'd need some work and for a first pass we could just mark the
framebuffer domain as always on and get most of the win.  Sometimes lack
of driver support is a good thing :)

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Tomasz Figa]

Hi,

W dniu 2 grudnia 2011 01:56 u?ytkownik Mark Brown
<broonie@opensource.wolfsonmicro.com> napisa?:
> On Fri, Dec 02, 2011 at 09:35:44AM +0900, Kyungmin Park wrote:
>
>> I'm not sure what's the next step at s3c64xx for generic power domain.
>> Related with exysno4 series, it's helpful to read following threads.
>> http://68.183.106.108/lists/linux-pm/msg26291.html
>
>> "I don't think we should control/gate the clocks with regarding power
>> domain" from Mr. Kim
>
> I tend to agree with him that gating the clocks automatically using
> runtime PM would be nice as if nothing else it saves code in drivers.
> I'm not sure how easy it's going to be to transition to doing that in
> one jump, though - there's a lot of SoCs sharing IP right back to
> S3C24xx.
>
> Of course it should be fairly easy for most of the domains on s3c64xx
> right now as there's only one actual driver involved in mainline, we
> should just be gating almost all of the clocks concerned immediately
> after we boot (assuming they aren't gated by default, I'd need to check)
> and never touching them again anyway.  It'd only be the framebuffer
> clocks that'd need some work and for a first pass we could just mark the
> framebuffer domain as always on and get most of the win.  Sometimes lack
> of driver support is a good thing :)

Please do not forget that there might be some drivers not yet submited
to mainline and mainline should not break them with an assumption that
there are no such drivers.

For example, there is an on-going work on an open source OpenGL
implementation for the GPU in S3C6410, known as OpenFIMG. Currently it
uses a little kernel module for low level hardware management
(interrupts, contexts, locking, power management), involving clock
gating and runtime power management, but a DRM driver is planned.

Best regards,
Tom 

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Mark Brown]

On Fri, Dec 02, 2011 at 07:25:01PM +0100, Tomasz Figa wrote:

> Please do not forget that there might be some drivers not yet submited
> to mainline and mainline should not break them with an assumption that
> there are no such drivers.

> For example, there is an on-going work on an open source OpenGL
> implementation for the GPU in S3C6410, known as OpenFIMG. Currently it
> uses a little kernel module for low level hardware management
> (interrupts, contexts, locking, power management), involving clock
> gating and runtime power management, but a DRM driver is planned.

As I said in the commit message for the patch you're following up on
I don't think that's a problem as it's pretty straightforward to add the
requisite hooks as part of adding the actual drivers.  The power domain
framework is really easy to use so it's not like it should cause real
effort, if you can figure out how to drive a 3D graphics engine adding a
device to a power domain shouldn't be too hard.

There will need to be arch/arm changes to add the platform devices and
hook up the clocks anyway so it's not like this is creating a need for
modifications.  Indeed I'd suggest adding the arch/arm stuff as it gets
written anyway, even if the actual drivers are still a work in progress.
It's one less thing to carry out of tree and it means that updates like
this or like the clk API reworks that I'd expect to start appearing soon
will happen for free.

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Tomasz Figa]

On Friday 02 of December 2011 18:57:43 Mark Brown wrote:
> On Fri, Dec 02, 2011 at 07:25:01PM +0100, Tomasz Figa wrote:
> > Please do not forget that there might be some drivers not yet submited
> > to mainline and mainline should not break them with an assumption that
> > there are no such drivers.
> > 
> > For example, there is an on-going work on an open source OpenGL
> > implementation for the GPU in S3C6410, known as OpenFIMG. Currently it
> > uses a little kernel module for low level hardware management
> > (interrupts, contexts, locking, power management), involving clock
> > gating and runtime power management, but a DRM driver is planned.
> 
> As I said in the commit message for the patch you're following up on
> I don't think that's a problem as it's pretty straightforward to add the
> requisite hooks as part of adding the actual drivers.  The power domain
> framework is really easy to use so it's not like it should cause real
> effort, if you can figure out how to drive a 3D graphics engine adding a
> device to a power domain shouldn't be too hard.
> 
> There will need to be arch/arm changes to add the platform devices and
> hook up the clocks anyway so it's not like this is creating a need for
> modifications.  Indeed I'd suggest adding the arch/arm stuff as it gets
> written anyway, even if the actual drivers are still a work in progress.
> It's one less thing to carry out of tree and it means that updates like
> this or like the clk API reworks that I'd expect to start appearing soon
> will happen for free.

Yeah, I'm okay with that, I just noted that there is more support for S3C6410 
available than just what is present in mainline.

Best regards,
Tom

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Sylwester Nawrocki]

Hi Mark,

good to see someone adding a proper power domain support for s3c64xx.

On 12/01/2011 07:48 PM, Mark Brown wrote:
> The S3C64xx SoCs contain a set of gateable power domains which can be
> enabled and disabled at runtime in order to save power.  Use the generic
> power domain code to implement support for these in software, enabling
> runtime control of most domains:
> 
>  - ETM (not supported in mainline).
>  - Domain G: 3D acceleration (no mainline support).
>  - Domain V: MFC (no mainline support).
>  - Domain I: JPEG and camera interface (no mainline support).
>  - Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
>  - Domain S: Security (no mainline support).
>  - Domain F: LCD (driver already uses runtime PM), post processing and
>    rotation (no mainline support).
> 
> The IROM domain is marked as always enabled as we should arrange for it
> to be enabled when we suspend which will need a bit more work.
> 
> Due to all the conditional device registration that the platform does
> wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device
> into the power domain after the machines have registered, looking for
> platform data to tell if the device was registered. Since currently only
> Cragganmore actually sets up PM that is the only machine updated.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
...
> +
> +static int s3c64xx_pd_on(struct generic_pm_domain *domain)
> +{
> +	struct s3c64xx_pm_domain *pd;
> +	u32 val;
> +	long retry = 1000000L;
> +
> +	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
> +
> +	val = __raw_readl(S3C64XX_NORMAL_CFG);
> +	val |= pd->ena;
> +	__raw_writel(val, S3C64XX_NORMAL_CFG);
> +
> +	/* Not all domains provide power status readback */
> +	if (pd->pwr_stat) {
> +		while (retry--)
> +			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
> +				break;

How about adding cpu_relax() in this busy wait loop ?

> +		if (!retry) {
> +			pr_err("Failed to start domain %s\n", pd->name);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	return 0;
> +}

--

Regards,
Sylwester

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Mark Brown]

On Fri, Dec 02, 2011 at 09:10:27PM +0100, Sylwester Nawrocki wrote:

> > +	/* Not all domains provide power status readback */
> > +	if (pd->pwr_stat) {
> > +		while (retry--)
> > +			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
> > +				break;

> How about adding cpu_relax() in this busy wait loop ?

May as well, yes - I didn't actually measure how long it tends to take
to do the spin but it's not going to hurt.

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Rafael J. Wysocki]

On Friday, December 02, 2011, Mark Brown wrote:
> On Fri, Dec 02, 2011 at 09:10:27PM +0100, Sylwester Nawrocki wrote:
> 
> > > +	/* Not all domains provide power status readback */
> > > +	if (pd->pwr_stat) {
> > > +		while (retry--)
> > > +			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
> > > +				break;
> 
> > How about adding cpu_relax() in this busy wait loop ?
> 
> May as well, yes - I didn't actually measure how long it tends to take
> to do the spin but it's not going to hurt.

Are you going to post an updated patch?

Rafael

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Mark Brown]

On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote:
> On Friday, December 02, 2011, Mark Brown wrote:

> > May as well, yes - I didn't actually measure how long it tends to take
> > to do the spin but it's not going to hurt.

> Are you going to post an updated patch?

I've resent it but my development laptop is currently not internet
connected so I'm not sure when it'll actually go out.

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Rafael J. Wysocki]

On Thursday, December 08, 2011, Mark Brown wrote:
> On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote:
> > On Friday, December 02, 2011, Mark Brown wrote:
> 
> > > May as well, yes - I didn't actually measure how long it tends to take
> > > to do the spin but it's not going to hurt.
> 
> > Are you going to post an updated patch?
> 
> I've resent it but my development laptop is currently not internet
> connected so I'm not sure when it'll actually go out.

Hmm.  I can't find it in my e-mail archives.

Do you have any pointer to a web archive somewhere, where I can get the patch
from?

Rafael

[PATCH 3/3] ARM: S3C64XX: Implement basic power domain support

[Rafael J. Wysocki]

On Thursday, December 08, 2011, Rafael J. Wysocki wrote:
> On Thursday, December 08, 2011, Mark Brown wrote:
> > On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote:
> > > On Friday, December 02, 2011, Mark Brown wrote:
> > 
> > > > May as well, yes - I didn't actually measure how long it tends to take
> > > > to do the spin but it's not going to hurt.
> > 
> > > Are you going to post an updated patch?
> > 
> > I've resent it but my development laptop is currently not internet
> > connected so I'm not sure when it'll actually go out.
> 
> Hmm.  I can't find it in my e-mail archives.
> 
> Do you have any pointer to a web archive somewhere, where I can get the patch
> from?

Ah, I've just received it.

Thanks,
Rafael

[PATCH 1/3] PM: Provide an always on power domain governor

[Rafael J. Wysocki]

On Thursday, December 01, 2011, Mark Brown wrote:
> Since systems are likely to have power domains that can't be turned off
> for various reasons at least temporarily while implementing power domain
> support provide a default governor which will always refuse to power off
> the domain, saving platforms having to implement their own.
> 
> Since the code is so tiny don't bother with a Kconfig symbol for it.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Looks good with the new version of [2/3].

Do you want me to takt it?

Rafael


> ---
>  drivers/base/power/domain_governor.c |   13 +++++++++++++
>  include/linux/pm_domain.h            |    2 ++
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index da78540..51527ee 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -141,3 +141,16 @@ struct dev_power_governor simple_qos_governor = {
>  	.stop_ok = default_stop_ok,
>  	.power_down_ok = default_power_down_ok,
>  };
> +
> +static bool always_on_power_down_ok(struct dev_pm_domain *domain)
> +{
> +	return false;
> +}
> +
> +/**
> + * pm_genpd_gov_always_on - A governor implementing an always-on policy
> + */
> +struct dev_power_governor pm_domain_always_on_gov = {
> +	.power_down_ok = always_on_power_down_ok,
> +	.stop_ok = default_stop_ok,
> +};
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index fbb81bc..fda14f1 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -139,6 +139,7 @@ extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
>  
>  extern bool default_stop_ok(struct device *dev);
>  
> +extern struct dev_power_governor pm_domain_always_on_gov;
>  #else
>  
>  static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
> @@ -192,6 +193,7 @@ static inline bool default_stop_ok(struct device *dev)
>  {
>  	return false;
>  }
> +#define pm_domain_always_on_gov NULL
>  #endif
>  
>  static inline int pm_genpd_remove_callbacks(struct device *dev)
> 

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

On Sun, Dec 04, 2011 at 09:56:59PM +0100, Rafael J. Wysocki wrote:
> On Thursday, December 01, 2011, Mark Brown wrote:

> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

> Looks good with the new version of [2/3].

> Do you want me to takt it?

It might make sense for it to go via the Samsung tree - the third patch
depends on it and there's some other stuff going on with the s3c64xx
power management - so let's see what Kukjin thinks is best.

[PATCH 1/3] PM: Provide an always on power domain governor

[Rafael J. Wysocki]

On Monday, December 05, 2011, Mark Brown wrote:
> On Sun, Dec 04, 2011 at 09:56:59PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, December 01, 2011, Mark Brown wrote:
> 
> > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> > Looks good with the new version of [2/3].
> 
> > Do you want me to takt it?
> 
> It might make sense for it to go via the Samsung tree - the third patch
> depends on it and there's some other stuff going on with the s3c64xx
> power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

On Mon, Dec 05, 2011 at 01:15:18AM +0100, Rafael J. Wysocki wrote:
> On Monday, December 05, 2011, Mark Brown wrote:

> > It might make sense for it to go via the Samsung tree - the third patch
> > depends on it and there's some other stuff going on with the s3c64xx
> > power management - so let's see what Kukjin thinks is best.

> OTOH, it will conflict with some patches I have in the works.

Ah, OK - in that case I guess it's sensible if you apply it.  Ideally it
could be on a separate branch so it could be cross-merged with other
trees if there's a need later.

[PATCH 1/3] PM: Provide an always on power domain governor

[Kukjin Kim]

Mark Brown wrote:
> 
> On Mon, Dec 05, 2011 at 01:15:18AM +0100, Rafael J. Wysocki wrote:
> > On Monday, December 05, 2011, Mark Brown wrote:
> 
> > > It might make sense for it to go via the Samsung tree - the third
> patch
> > > depends on it and there's some other stuff going on with the s3c64xx
> > > power management - so let's see what Kukjin thinks is best.
> 
> > OTOH, it will conflict with some patches I have in the works.
> 
> Ah, OK - in that case I guess it's sensible if you apply it.  Ideally it
> could be on a separate branch so it could be cross-merged with other
> trees if there's a need later.

Hi Mark and Rafael,

Looks ok to me on this series and would be nice to me if Rafael could create
topic branch in his tree and apply this series with my ack for Samsung
stuff.

Rafael, please let me know the branch when you create it so that I can merge
it into Samsung tree to avoid other conflicts.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/3] PM: Provide an always on power domain governor

[Marek Szyprowski]

Hello,

On Tuesday, December 06, 2011 11:20 AM Kukjin Kim wrote:
 
> Mark Brown wrote:
> >
> > On Mon, Dec 05, 2011 at 01:15:18AM +0100, Rafael J. Wysocki wrote:
> > > On Monday, December 05, 2011, Mark Brown wrote:
> >
> > > > It might make sense for it to go via the Samsung tree - the third
> > patch
> > > > depends on it and there's some other stuff going on with the s3c64xx
> > > > power management - so let's see what Kukjin thinks is best.
> >
> > > OTOH, it will conflict with some patches I have in the works.
> >
> > Ah, OK - in that case I guess it's sensible if you apply it.  Ideally it
> > could be on a separate branch so it could be cross-merged with other
> > trees if there's a need later.
> 
> Hi Mark and Rafael,
> 
> Looks ok to me on this series and would be nice to me if Rafael could create
> topic branch in his tree and apply this series with my ack for Samsung
> stuff.
> 
> Rafael, please let me know the branch when you create it so that I can merge
> it into Samsung tree to avoid other conflicts.

What about similar patches for S5PV210/S5PC110 series and Exynos4? gen_pd based
power domain code for Exynos4 have been rejected in favor of custom platform-device
based power domain drivers. It would be much easier to have only one type of the
drivers across different Samsung SoCs. This will also help to hide some differences
between the series from the device drivers (clock gating, power management). 
 
Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

On Tue, Dec 06, 2011 at 01:04:53PM +0100, Marek Szyprowski wrote:

Please fix your mailer to word wrap at less than 80 columns, I've
reflowed your text for legibility.

> What about similar patches for S5PV210/S5PC110 series and Exynos4?

I don't really have access to these systems (well, I have a Nexus S I
can use but that doesn't boot mainline) and I don't have datasheets so
it's hard for me to actively work on them myself.

> gen_pd based power domain code for Exynos4 have been rejected in favor
> of custom platform-device based power domain drivers. It would be much

Oh, that's very surprising to me - looking at the code it appeared that
it just predated gen_pd, I didn't see anything in there that suggested
an active decision to avoid using it.  What was then thinking there?  Is
this something we can fix by enhancing gen_pd?

> easier to have only one type of the drivers across different Samsung
> SoCs. This will also help to hide some differences between the series
> from the device drivers (clock gating, power management). 

When I looked at the code I'd expected the other SoCs to also transition
over to gen_pd and then start pulling things out from there.

Looking at what you can do on s3c64xx the power domain code looked very
small once you use the gen_pd stuff (which is very nice to use, it
really makes the whole thing very easy) so I'm not sure how much win it
is immediately.  How much win it is long term will depend on where
things get implemented - a lot of the stuff I could tink of doing seemed
like it might be more of a fit for the opp or PM QoS stuff than for the
power domains, with the power domains staying pretty thin and just
providing inputs but perhaps that's not a sensible approach.

The main trick is going to be actually putting the devices into the
power domains which is entertaining as currently every board directly
registers platform devices so it's hard for core code to work out what
Linux knows about.

In terms of the drivers I wasn't expecting to see much impact as the
power domain and runtime PM APIs provide a pretty good abstraction
already - the actual implementation of things is totally transparent to
them.  The only device actively able to use the power domains on the
S3C6410 is the framebuffer and the driver there just worked with the
existing runtime PM code.

[PATCH 1/3] PM: Provide an always on power domain governor

[Rafael J. Wysocki]

On Tuesday, December 06, 2011, Kukjin Kim wrote:
> Mark Brown wrote:
> > 
> > On Mon, Dec 05, 2011 at 01:15:18AM +0100, Rafael J. Wysocki wrote:
> > > On Monday, December 05, 2011, Mark Brown wrote:
> > 
> > > > It might make sense for it to go via the Samsung tree - the third
> > patch
> > > > depends on it and there's some other stuff going on with the s3c64xx
> > > > power management - so let's see what Kukjin thinks is best.
> > 
> > > OTOH, it will conflict with some patches I have in the works.
> > 
> > Ah, OK - in that case I guess it's sensible if you apply it.  Ideally it
> > could be on a separate branch so it could be cross-merged with other
> > trees if there's a need later.
> 
> Hi Mark and Rafael,
> 
> Looks ok to me on this series and would be nice to me if Rafael could create
> topic branch in his tree and apply this series with my ack for Samsung
> stuff.
> 
> Rafael, please let me know the branch when you create it so that I can merge
> it into Samsung tree to avoid other conflicts.

I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll
put patches [1/3] and [2/3] (new version) into my linux-next branch and then
(after a few days spent in linux-next) to the pm-domains branch.  I'll let you
know when that happens.

Thanks,
Rafael

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

On Tue, Dec 06, 2011 at 10:39:56PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, December 06, 2011, Kukjin Kim wrote:

> > Rafael, please let me know the branch when you create it so that I can merge
> > it into Samsung tree to avoid other conflicts.

> I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll
> put patches [1/3] and [2/3] (new version) into my linux-next branch and then
> (after a few days spent in linux-next) to the pm-domains branch.  I'll let you
> know when that happens.

Well, I guess you could put patch 1 and 3 in then add 2 if an ack is
forthcoming for that - there's no direct dependency.

[PATCH 1/3] PM: Provide an always on power domain governor

[Rafael J. Wysocki]

On Wednesday, December 07, 2011, Mark Brown wrote:
> On Tue, Dec 06, 2011 at 10:39:56PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, December 06, 2011, Kukjin Kim wrote:
> 
> > > Rafael, please let me know the branch when you create it so that I can merge
> > > it into Samsung tree to avoid other conflicts.
> 
> > I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll
> > put patches [1/3] and [2/3] (new version) into my linux-next branch and then
> > (after a few days spent in linux-next) to the pm-domains branch.  I'll let you
> > know when that happens.
> 
> Well, I guess you could put patch 1 and 3 in then add 2 if an ack is
> forthcoming for that - there's no direct dependency.

It's been acked already, I'm going to pul all three patches into
linux-pm/linux-next later today.

Thanks,
Rafael

[PATCH 1/3] PM: Provide an always on power domain governor

[Mark Brown]

On Wed, Dec 07, 2011 at 10:28:02PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, December 07, 2011, Mark Brown wrote:

> > Well, I guess you could put patch 1 and 3 in then add 2 if an ack is
> > forthcoming for that - there's no direct dependency.

> It's been acked already, I'm going to pul all three patches into
> linux-pm/linux-next later today.

Oh, OK - I didn't seem to get copied on any of the acks or at least I
didn't see them.