[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[MyungJoo Ham]

This patches add support for powerdomain, block-gating, and flags in struct clk.

Blockgating re-uses powerdomain support scheme and depends on powerdomain support.

Flags support is independent from powerdomain; however, powerdomain support is NOT stable without flags support. Without flags support, powerdomain may lock up the system with some conditions although they are rare. Thus, when powerdomain or block-gating is used, flags support should be turned on for the system stability. (and that's why I'm sending flags support with powerdomain/block-gating support).

Although powerdomain support is requred for blockgating, blockgating is NOT required for powerdomain. Besides, powerdomain support is observed to reduce power consumption significantly (with 2.6.29 kernel); however, blockgating support didn't show any significant improvement.


MyungJoo Ham (4):
  ARM: SAMSUNG SoC: Powerdomain/Block-gating Support
  ARM: S5PV210: Powerdomain/Clock-gating Support
  ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.
  ARM: S5PV210: Clock Framework: Flag Support for struct clk.

 arch/arm/mach-s5pv210/Kconfig              |   17 +
 arch/arm/mach-s5pv210/clock.c              |  906 ++++++++++++++++++++++------
 arch/arm/plat-samsung/Kconfig              |   19 +
 arch/arm/plat-samsung/clock.c              |  146 +++++-
 arch/arm/plat-samsung/include/plat/clock.h |   44 ++
 5 files changed, 935 insertions(+), 197 deletions(-)

[PATCH 1/4] ARM: SAMSUNG SoC: Powerdomain/Block-gating Support

[MyungJoo Ham]

Even when all the clocks of a domain are turned off, turning off the
power for the domain saves (leakage) current. Thus, we need to control
powerdomain accordingly to save even more power when we do clock gating.

Block-gating is a similar feature with powerdomain, but, it controls
"block", which is smaller than power-domain, and the saved current is
not so significant.

This patch enables powerdomain/block-gating support for Samsung SoC.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/Kconfig              |   19 +++++++
 arch/arm/plat-samsung/clock.c              |   76 ++++++++++++++++++++++++++-
 arch/arm/plat-samsung/include/plat/clock.h |   32 ++++++++++++
 3 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index bd007e3..1fddb04 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -298,4 +298,23 @@ config SAMSUNG_WAKEMASK
 	  and above. This code allows a set of interrupt to wakeup-mask
 	  mappings. See <plat/wakeup-mask.h>
 
+config SAMSUNG_POWERDOMAIN
+	bool "Powerdomain Control Support"
+	depends on CPU_S5PV210
+	select S5PV210_POWERDOMAIN
+	help
+	  Compile support for powerdomain controls. This code allows
+	  enabling and diabling powerdomain according to its clocks,
+	  which often saves significant amount of power.
+
+config SAMSUNG_BLOCKGATING
+	bool "Block Gating Control Support"
+	depends on SAMSUNG_POWERDOMAIN && CPU_S5PV210
+	select S5PV210_BLOCKGATING
+	help
+	  Compile support for block gating controls. This code allows
+	  enabling and diabling blocks according to its clocks.
+	  However, at least in S5PV210, this is not as effective as
+	  powerdomain control.
+
 endif
diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c
index 8bf79f3..f8a30f7 100644
--- a/arch/arm/plat-samsung/clock.c
+++ b/arch/arm/plat-samsung/clock.c
@@ -113,6 +113,10 @@ void clk_put(struct clk *clk)
 
 int clk_enable(struct clk *clk)
 {
+#if defined(CONFIG_SAMSUNG_BLOCKGATING) || defined(CONFIG_SAMSUNG_POWERDOMAIN)
+	struct clk *p;
+#endif
+
 	if (IS_ERR(clk) || clk == NULL)
 		return -EINVAL;
 
@@ -120,8 +124,38 @@ int clk_enable(struct clk *clk)
 
 	spin_lock(&clocks_lock);
 
-	if ((clk->usage++) == 0)
-		(clk->enable)(clk, 1);
+	if ((clk->usage++) == 0) {
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+		if (clk->bd && clk->bd->ref_count == 0) {
+			if (clk->bd->pd_set)
+				clk->bd->pd_set(clk->bd, 1);
+			clk->bd->ref_count++;
+		}
+#endif
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+		if (clk->pd) {
+			if (clk->pd->ref_count == 0 &&
+					clk->pd->num_clks_boot_on <= 0) {
+				list_for_each_entry(p, &clk->pd->clocks,
+						powerdomain_list) {
+					if (p->enable)
+						p->enable(p, 1);
+				}
+
+				clk->pd->pd_set(clk->pd, 1);
+
+				list_for_each_entry(p, &clk->pd->clocks,
+						powerdomain_list) {
+					if (p->enable)
+						p->enable(p, 0);
+				}
+			}
+			clk->pd->ref_count++;
+		}
+#endif
+		if (clk->enable)
+			(clk->enable)(clk, 1);
+	}
 
 	spin_unlock(&clocks_lock);
 	return 0;
@@ -134,8 +168,29 @@ void clk_disable(struct clk *clk)
 
 	spin_lock(&clocks_lock);
 
-	if ((--clk->usage) == 0)
+	if ((--clk->usage) == 0) {
 		(clk->enable)(clk, 0);
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+		if (clk->pd) {
+			if (clk->pd->ref_count == 1 &&
+					clk->pd->num_clks_boot_on <= 0 &&
+					clk->pd->pd_set)
+				clk->pd->pd_set(clk->pd, 0);
+			if (clk->pd->ref_count > 0)
+				clk->pd->ref_count--;
+		}
+#endif
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+		if (clk->bd) {
+			if (clk->bd->ref_count == 1 &&
+					clk->bd->num_clks_boot_on <= 0 &&
+					clk->bd->pd_set)
+				clk->bd->pd_set(clk->bd, 0);
+			if (clk->bd->ref_count > 0)
+				clk->bd->ref_count--;
+		}
+#endif
+	}
 
 	spin_unlock(&clocks_lock);
 	clk_disable(clk->parent);
@@ -327,6 +382,21 @@ int s3c24xx_register_clock(struct clk *clk)
 	list_add(&clk->list, &clocks);
 	spin_unlock(&clocks_lock);
 
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+	if (clk->pd) {
+		spin_lock(&clocks_lock);
+		list_add(&clk->powerdomain_list, &clk->pd->clocks);
+		spin_unlock(&clocks_lock);
+	}
+#endif
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+	if (clk->bd) {
+		spin_lock(&clocks_lock);
+		list_add(&clk->blockgating_list, &clk->bd->clocks);
+		spin_unlock(&clocks_lock);
+	}
+#endif
+
 	return 0;
 }
 
diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-samsung/include/plat/clock.h
index 0fbcd0e..bf851e1 100644
--- a/arch/arm/plat-samsung/include/plat/clock.h
+++ b/arch/arm/plat-samsung/include/plat/clock.h
@@ -11,6 +11,30 @@
 
 #include <linux/spinlock.h>
 
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+struct register_save {
+	unsigned int		addr;
+	unsigned int		data;
+};
+
+struct powerdomain {
+	/* The number of clks that are never-disabled and "BOOT_ON" */
+	int			num_clks_boot_on;
+	struct list_head	clocks;
+
+	unsigned long	*pd_reg;
+	unsigned long	*pd_stable_reg;
+	unsigned long		pd_ctrlbit;
+	unsigned long		pd_stable_ctrlbit;
+	int			ref_count;
+
+	int			(*pd_set)(struct powerdomain *, int enable);
+	bool			normal_ff_saved;
+	struct register_save	*normal_ff;
+	unsigned int		num_normal_ff;
+};
+#endif
+
 struct clk;
 
 /**
@@ -47,6 +71,14 @@ struct clk {
 
 	struct clk_ops		*ops;
 	int		    (*enable)(struct clk *, int enable);
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+	struct powerdomain	*pd;
+	struct list_head	powerdomain_list;
+#endif
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+	struct powerdomain	*bd;
+	struct list_head	blockgating_list;
+#endif
 };
 
 /* other clocks which may be registered by board support */
-- 
1.6.3.3

[PATCH 2/4] ARM: S5PV210: Powerdomain/Clock-gating Support

[MyungJoo Ham]

S5PV210 Machine specific code for powerdomain/clock-gating support

Powerdomains and Blocks are defined for each corresponding clk.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s5pv210/Kconfig |   17 ++
 arch/arm/mach-s5pv210/clock.c |  461 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 478 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig
index 631019a..92f6b74 100644
--- a/arch/arm/mach-s5pv210/Kconfig
+++ b/arch/arm/mach-s5pv210/Kconfig
@@ -101,4 +101,21 @@ config MACH_SMDKC110
 	  Machine support for Samsung SMDKC110
 	  S5PC110(MCP) is one of package option of S5PV210
 
+config S5PV210_POWERDOMAIN
+	bool "Use Powerdomain control for S5PV210"
+	depends on SAMSUNG_POWERDOMAIN && CPU_S5PV210
+	help
+	  Compile support for powerdomain controls in S5PV210.
+	  This code allows enabling and diabling powerdomain
+	  according to its clocks, which often saves significant
+	  amount of power. Note that BLOCKGATING code depends on
+	  POWERDOMAIN in S5PV210 code.
+
+config S5PV210_BLOCKGATING
+	bool "Use Block Gating for S5PV210"
+	depends on SAMSUNG_POWERDOMAIN && S5PV210_POWERDOMAIN &&\
+		SAMSUNG_BLOCKGATING
+	help
+	  Compile support for block gating controls in S5PV210.
+
 endif
diff --git a/arch/arm/mach-s5pv210/clock.c b/arch/arm/mach-s5pv210/clock.c
index 04a0ef9..af2af5e 100644
--- a/arch/arm/mach-s5pv210/clock.c
+++ b/arch/arm/mach-s5pv210/clock.c
@@ -19,6 +19,7 @@
 #include <linux/clk.h>
 #include <linux/sysdev.h>
 #include <linux/io.h>
+#include <linux/spinlock.h>
 
 #include <mach/map.h>
 
@@ -31,6 +32,392 @@
 #include <plat/clock-clksrc.h>
 #include <plat/s5pv210.h>
 
+#ifdef CONFIG_S5PV210_POWERDOMAIN
+#include <linux/delay.h>
+#define PD_SET(x)		.pd = &pd_##x,
+
+#ifdef CONFIG_S5PV210_BLOCKGATING
+#define BD_SET(x)		.bd = &bd_##x,
+#else
+#define BD_SET(x)
+#endif
+
+#else
+#define PD_SET(x)
+#define BD_SET(x)
+#endif
+
+#ifdef CONFIG_S5PV210_POWERDOMAIN
+static void normal_ff_save(struct register_save *regs, unsigned int num_regs)
+{
+	int i;
+	for (i = 0; i < num_regs; i++)
+		regs[i].data = __raw_readl(regs[i].addr);
+}
+
+static void normal_ff_restore(struct register_save *regs, unsigned int num_regs)
+{
+	int i;
+	for (i = 0; i < num_regs; i++) {
+		__raw_writel(regs[i].data, regs[i].addr);
+#ifdef CONFIG_S5PC110_EVT0_WORKAROUND
+		__raw_readl(regs[i].addr);
+#endif
+	}
+}
+
+DEFINE_SPINLOCK(powerdomain_lock);
+
+static int powerdomain_set(struct powerdomain *pd, int enable)
+{
+	unsigned long ctrlbit;
+	void __iomem *reg;
+	void __iomem *stable_reg;
+	unsigned long reg_dat;
+
+	if (pd == NULL)
+		return -EINVAL;
+
+	ctrlbit = pd->pd_ctrlbit;
+	reg = (void __iomem *)pd->pd_reg;
+	stable_reg = (void __iomem *)pd->pd_stable_reg;
+
+	spin_lock(&powerdomain_lock);
+	reg_dat = __raw_readl(reg);
+
+	if (enable) {
+		__raw_writel(reg_dat|ctrlbit, reg);
+		spin_unlock(&powerdomain_lock);
+
+		if (pd->pd_stable_ctrlbit && stable_reg) {
+			do { } while (!(__raw_readl(stable_reg) &
+						pd->pd_stable_ctrlbit));
+		}
+
+		if (pd->normal_ff_saved && pd->normal_ff) {
+			normal_ff_restore(pd->normal_ff, pd->num_normal_ff);
+			pd->normal_ff_saved = 0;
+		}
+	} else {
+		if (pd->normal_ff) {
+			normal_ff_save(pd->normal_ff, pd->num_normal_ff);
+			pd->normal_ff_saved = 1;
+		}
+		__raw_writel(reg_dat & ~(ctrlbit), reg);
+		spin_unlock(&powerdomain_lock);
+	}
+
+	return 0;
+}
+
+static struct powerdomain pd_lcd = {
+	/* L-block: G2D, FIMD, MIE, DSIM */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 3),
+	.pd_stable_ctrlbit	= (0x1 << 3),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_lcd.clocks),
+};
+
+static struct powerdomain pd_tv = {
+	/* T-block: VP, MIXER, TVENC, HDMI */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 4),
+	.pd_stable_ctrlbit	= (0x1 << 4),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_tv.clocks),
+};
+
+static struct powerdomain pd_mfc = {
+	/* F-block: MFC */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 1),
+	.pd_stable_ctrlbit	= (0x1 << 1),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_mfc.clocks),
+};
+
+static struct powerdomain pd_cam = {
+	/* X-block: FIMC0, FIMC1, FIMC2, CSIS, JPEG, Rotator,
+	 * MIE(Deprecated) */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 5),
+	.pd_stable_ctrlbit	= (0x1 << 5),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_cam.clocks),
+};
+
+static struct powerdomain pd_audio = {
+	/* Audio sub-block: I2S0, Audio buffer RAM */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 7),
+	.pd_stable_ctrlbit	= (0x1 << 7),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_audio.clocks),
+};
+
+static struct powerdomain pd_irom = {
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= (0x1 << 20),
+	.pd_stable_ctrlbit	= (0x0), /* not avaiable */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_irom.clocks),
+};
+
+static struct powerdomain pd_g3d = {
+	/* G3D Block */
+	.pd_reg			= S5P_NORMAL_CFG,
+	.pd_stable_reg		= S5P_BLK_PWR_STAT,
+	.pd_ctrlbit		= (0x1 << 2),
+	.pd_stable_ctrlbit	= (0x1 << 2),
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(pd_g3d.clocks),
+};
+
+static struct powerdomain *pd_list[] = {
+	&pd_lcd,
+	&pd_tv,
+	&pd_mfc,
+	&pd_cam,
+	&pd_audio,
+	&pd_irom,
+	&pd_g3d,
+};
+
+/* Support for Deep-Idle:
+ * AUDIO block is not turned off with top-off (deep-idle) */
+static struct powerdomain *pd_backup_topoff[] = {
+	&pd_lcd,
+	&pd_tv,
+	&pd_mfc,
+	&pd_cam,
+	&pd_irom,
+	&pd_g3d,
+};
+
+unsigned int powerdomain_backup_topoff(void)
+{
+	int pdset = 0;
+	int i;
+	for (i = 0; i < ARRAY_SIZE(pd_backup_topoff); i++) {
+		if (__raw_readl(pd_backup_topoff[i]->pd_reg) &
+				pd_backup_topoff[i]->pd_ctrlbit) {
+			/* powerdomain alive. back it up */
+			normal_ff_save(pd_backup_topoff[i]->normal_ff,
+					pd_backup_topoff[i]->num_normal_ff);
+			pdset |= pd_backup_topoff[i]->pd_ctrlbit;
+		}
+	}
+	return pdset;
+}
+
+void powerdomain_restore_topoff(unsigned int pdset)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pd_backup_topoff); i++) {
+		if (pdset & pd_backup_topoff[i]->pd_ctrlbit) {
+			/* powerdomain backuped. restore it */
+			normal_ff_restore(pd_backup_topoff[i]->normal_ff,
+					pd_backup_topoff[i]->num_normal_ff);
+		}
+	}
+}
+#endif
+
+#ifdef CONFIG_S5PV210_BLOCKGATING
+static struct powerdomain bd_intc = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 10,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_intc.clocks),
+};
+
+static struct powerdomain bd_hsmmc = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 9,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_hsmmc.clocks),
+};
+
+static struct powerdomain bd_debug = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 8,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_debug.clocks),
+};
+
+static struct powerdomain bd_security = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 7,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_security.clocks),
+};
+
+static struct powerdomain bd_memory = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 6,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_memory.clocks),
+};
+
+static struct powerdomain bd_usb = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 5,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_usb.clocks),
+};
+
+static struct powerdomain bd_tv = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 4,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_tv.clocks),
+};
+
+static struct powerdomain bd_lcd = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 3,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_lcd.clocks),
+};
+
+static struct powerdomain bd_img = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 2,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_img.clocks),
+};
+
+static struct powerdomain bd_mfc = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 1,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_mfc.clocks),
+};
+
+static struct powerdomain bd_g3d = {
+	.pd_reg			= S5P_CLKGATE_BLOCK,
+	.pd_stable_reg		= NULL,
+	.pd_ctrlbit		= 1 << 0,
+	.pd_stable_ctrlbit	= (0x0), /* not available */
+	.ref_count		= 0,
+	.pd_set			= powerdomain_set,
+	.normal_ff_saved	= 0,
+	.normal_ff		= NULL,
+	.num_normal_ff		= 0,
+	.clocks			= LIST_HEAD_INIT(bd_g3d.clocks),
+};
+
+static struct powerdomain *bd_list[] = {
+	&bd_intc,
+	&bd_hsmmc,
+	&bd_debug,
+	&bd_security,
+	&bd_memory,
+	&bd_usb,
+	&bd_tv,
+	&bd_lcd,
+	&bd_img,
+	&bd_mfc,
+	&bd_g3d,
+};
+#endif
+
 static struct clksrc_clk clk_mout_apll = {
 	.clk	= {
 		.name		= "mout_apll",
@@ -280,138 +667,173 @@ static struct clk init_clocks_disable[] = {
 		.parent		= &clk_pclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1 << 31),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "rot",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1<<29),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "jpeg",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1 << 28),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "fimc",
 		.id		= 0,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1 << 24),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "fimc",
 		.id		= 1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1 << 25),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "fimc",
 		.id		= 2,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip0_ctrl,
 		.ctrlbit	= (1 << 26),
+		PD_SET(cam)
+		BD_SET(img)
 	}, {
 		.name		= "nfcon",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 28),
+		BD_SET(memory)
 	}, {
 		.name		= "tvenc",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 10),
+		PD_SET(tv)
+		BD_SET(tv)
 	}, {
 		.name		= "hdmi",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 11),
+		PD_SET(tv)
+		BD_SET(tv)
 	}, {
 		.name		= "mixer",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 9),
+		PD_SET(tv)
+		BD_SET(tv)
 	}, {
 		.name		= "otg",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<16),
+		BD_SET(usb)
 	}, {
 		.name		= "usb-host",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<17),
+		BD_SET(usb)
 	}, {
 		.name		= "lcd",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<0),
+		PD_SET(lcd)
+		BD_SET(lcd)
 	}, {
 		.name		= "cfcon",
 		.id		= 0,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<25),
+		BD_SET(memory)
 	}, {
 		.name		= "vp",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 8),
+		PD_SET(tv)
+		BD_SET(tv)
 	}, {
 		.name		= "dsim",
 		.id		= -1,
 		.parent		= &clk_hclk_dsys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 2),
+		PD_SET(lcd)
+		BD_SET(lcd)
 	}, {
 		.name		= "hsmmc",
 		.id		= 0,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<16),
+		BD_SET(hsmmc)
 	}, {
 		.name		= "hsmmc",
 		.id		= 1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<17),
+		BD_SET(hsmmc)
 	}, {
 		.name		= "hsmmc",
 		.id		= 2,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<18),
+		BD_SET(hsmmc)
 	}, {
 		.name		= "hsmmc",
 		.id		= 3,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<19),
+		BD_SET(hsmmc)
 	}, {
 		.name		= "tsi",
 		.id		= -1,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 20),
+		BD_SET(hsmmc)
 	}, {
 		.name		= "hostif",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 10),
+		BD_SET(debug)
 	}, {
 		.name		= "modem",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 9),
+		BD_SET(debug)
 	}, {
 		.name		= "pcm",
 		.id		= 0,
@@ -520,6 +942,8 @@ static struct clk init_clocks_disable[] = {
 		.parent		= &clk_p,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<4),
+		/* Note that only i2s-0 is connected to pd_audio */
+		PD_SET(audio)
 	}, {
 		.name		= "i2s_v32",
 		.id		= 0,
@@ -614,72 +1038,84 @@ static struct clk init_clocks[] = {
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 24),
+		BD_SET(memory)
 	}, {
 		.name		= "sromc",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 26),
+		BD_SET(memory)
 	}, {
 		.name		= "tzic",
 		.id		= 0,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 28),
+		BD_SET(intc)
 	}, {
 		.name		= "tzic",
 		.id		= 1,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 29),
+		BD_SET(intc)
 	}, {
 		.name		= "tzic",
 		.id		= 2,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 30),
+		BD_SET(intc)
 	}, {
 		.name		= "tzic",
 		.id		= 3,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 31),
+		BD_SET(intc)
 	}, {
 		.name		= "vic",
 		.id		= 0,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 24),
+		BD_SET(intc)
 	}, {
 		.name		= "vic",
 		.id		= 1,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 25),
+		BD_SET(intc)
 	}, {
 		.name		= "vic",
 		.id		= 2,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 26),
+		BD_SET(intc)
 	}, {
 		.name		= "vic",
 		.id		= 3,
 		.parent		= &clk_hclk_msys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 27),
+		BD_SET(intc)
 	}, {
 		.name		= "secjtag",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 11),
+		BD_SET(debug)
 	}, {
 		.name		= "coresight",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 8),
+		BD_SET(debug)
 	}, {
 		.name		= "sdm",
 		.id		= -1,
@@ -691,6 +1127,7 @@ static struct clk init_clocks[] = {
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 0),
+		BD_SET(security)
 	}, {
 		.name		= "syscon",
 		.id		= -1,
@@ -1344,5 +1781,29 @@ void __init s5pv210_register_clocks(void)
 		(clkp->enable)(clkp, 0);
 	}
 
+#ifdef CONFIG_S5PV210_POWERDOMAIN
+	/* According to the state of the members, turn on/off power domains
+	 * and blocks. */
+	for (ptr = 0; ptr < ARRAY_SIZE(pd_list); ptr++) {
+		if (pd_list[ptr]->pd_set) {
+			if (pd_list[ptr]->num_clks_boot_on > 0)
+				pd_list[ptr]->pd_set(pd_list[ptr], 1);
+			else
+				pd_list[ptr]->pd_set(pd_list[ptr], 0);
+		}
+	}
+#endif /* CONFIG_S5PV210_POWERDOMAIN */
+
+#ifdef CONFIG_S5PV210_BLOCKGATING
+	for (ptr = 0; ptr < ARRAY_SIZE(bd_list); ptr++) {
+		if (bd_list[ptr]->pd_set) {
+			if (bd_list[ptr]->num_clks_boot_on > 0)
+				bd_list[ptr]->pd_set(bd_list[ptr], 1);
+			else
+				bd_list[ptr]->pd_set(bd_list[ptr], 0);
+		}
+	}
+#endif /* CONFIG_S5PV210_BLOCKGATING */
+
 	s3c_pwmclk_init();
 }
-- 
1.6.3.3

[PATCH 3/4] ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.

[MyungJoo Ham]

Add a .flag property to the struct clk. The flag can have the following
information:

	The clock is enabled at the boot time.
	The clock cannot be disabled.
	The clock is enabled at the boot time and cannot be disabled.
	The clock is disabled at the boot time. Note that both
CLKFLAGS_BOOT_ON and CLKFLAGS_CANNOT_DISABLE can override
CLKFLAGS_BOOT_OFF.
	Please stop using this clock. When a DEPRECATED clock is
clk_get'd, clk_get function will printk a warning message.

The previous patch related with powerdomain / blcok-gating control
requires this patch for the stability related with clocks that are
turned on at the boot time.

Note that clocks without both BOOT_ON and BOOT_OFF keep the previous
states; however, powerdomain / block-gating controls do NOT have any
information about the states of such clocks, which in turn, may incur
instable kernel behaviors. For example, a powerdomain may be turned off
while a clock of the domain is still being used. With the flag support
feature, we highly recommend to define .flag fields fully for clocks
related to powerdomain and block-gating. Clocks not connected to
powerdomain and block-gating are ok without flag field.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/clock.c              |   70 ++++++++++++++++++++++++++-
 arch/arm/plat-samsung/include/plat/clock.h |   12 +++++
 2 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c
index f8a30f7..9ff9d63 100644
--- a/arch/arm/plat-samsung/clock.c
+++ b/arch/arm/plat-samsung/clock.c
@@ -103,6 +103,14 @@ struct clk *clk_get(struct device *dev, const char *id)
 	}
 
 	spin_unlock(&clocks_lock);
+
+	if (!IS_ERR(clk) && clk)
+		if (clk->flags & CLKFLAGS_DEPRECATED)
+			printk(KERN_WARNING "[%s:%d] clk %s:%d is deprecated. "
+					"Please reconsider using it.\n",
+					__FILE__, __LINE__, clk->name,
+					clk->id);
+
 	return clk;
 }
 
@@ -169,7 +177,25 @@ void clk_disable(struct clk *clk)
 	spin_lock(&clocks_lock);
 
 	if ((--clk->usage) == 0) {
-		(clk->enable)(clk, 0);
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+		if ((clk->flags | CLKFLAGS_BOOT_ON) &&
+				!(clk->flags | CLKFLAGS_CANNOT_DISABLE)) {
+			/* BOOT_ON became NO EFFECT. Let PD/BD be able
+			 * to turn themselves off */
+
+			if (clk->pd)
+				clk->pd->num_clks_boot_on--;
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+			if (clk->bd)
+				clk->bd->num_clks_boot_on--;
+#endif
+			clk->flags &= ~CLKFLAGS_BOOT_ON;
+		}
+#endif
+
+		if (clk->enable && !(clk->flags | CLKFLAGS_CANNOT_DISABLE))
+			(clk->enable)(clk, 0);
+
 #ifdef CONFIG_SAMSUNG_POWERDOMAIN
 		if (clk->pd) {
 			if (clk->pd->ref_count == 1 &&
@@ -193,7 +219,9 @@ void clk_disable(struct clk *clk)
 	}
 
 	spin_unlock(&clocks_lock);
-	clk_disable(clk->parent);
+
+	if (!(clk->flags | CLKFLAGS_CANNOT_DISABLE))
+		clk_disable(clk->parent);
 }
 
 
@@ -370,6 +398,13 @@ struct clk s3c24xx_uclk = {
  */
 int s3c24xx_register_clock(struct clk *clk)
 {
+	int ret = 0;
+
+	if (clk == NULL)
+		return -EINVAL;
+	if (clk->name == NULL)
+		return -EINVAL;
+
 	if (clk->enable == NULL)
 		clk->enable = clk_null_enable;
 
@@ -382,6 +417,21 @@ int s3c24xx_register_clock(struct clk *clk)
 	list_add(&clk->list, &clocks);
 	spin_unlock(&clocks_lock);
 
+	if (clk->flags & CLKFLAGS_BOOT_ON) {
+		/* Use clk_enable, not clk->enable as clk's parent is
+		 * also required to be turned on */
+		clk_enable(clk);
+
+#ifdef CONFIG_SAMSUNG_POWERDOMAIN
+		if (clk->pd)
+			clk->pd->num_clks_boot_on++;
+#endif
+#ifdef CONFIG_SAMSUNG_BLOCKGATING
+		if (clk->bd)
+			clk->bd->num_clks_boot_on++;
+#endif
+	}
+
 #ifdef CONFIG_SAMSUNG_POWERDOMAIN
 	if (clk->pd) {
 		spin_lock(&clocks_lock);
@@ -397,7 +447,21 @@ int s3c24xx_register_clock(struct clk *clk)
 	}
 #endif
 
-	return 0;
+	if (clk->flags & CLKFLAGS_BOOT_OFF) {
+		if ((clk->flags & CLKFLAGS_BOOT_ON) ||
+				(clk->flags & CLKFLAGS_CANNOT_DISABLE)) {
+			printk(KERN_WARNING "[%s:%d] clk %s:%d has incompatible"
+					" initilization flags: %x.\n",
+					__FILE__, __LINE__, clk->name, clk->id,
+					clk->flags);
+			ret = -EINVAL;
+		} else
+			/* Do NOT use clk_disable(clk) because clk_disable
+			 * may disable clk's parent */
+			ret = (clk->enable)(clk, 0);
+	}
+
+	return ret;
 }
 
 /**
diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-samsung/include/plat/clock.h
index bf851e1..0f7d556 100644
--- a/arch/arm/plat-samsung/include/plat/clock.h
+++ b/arch/arm/plat-samsung/include/plat/clock.h
@@ -59,6 +59,17 @@ struct clk_ops {
 	int		    (*set_parent)(struct clk *c, struct clk *parent);
 };
 
+/* Note that when BOOT_ON and OFF are both not set, the kernel
+ * keeps the state initialized by the boot-loader or cpu.
+ */
+#define CLKFLAGS_BOOT_ON	(0x1)
+#define CLKFLAGS_CANNOT_DISABLE	(0x2)
+#define CLKFLAGS_ALWAYS_ON	(CLKFLAGS_BOOT_ON | CLKFLAGS_CANNOT_DISABLE)
+#define CLKFLAGS_BOOT_OFF	(0x4) /* Force off when boot */
+#define CLKFLAGS_DEPRECATED	(0x8) /* Warn when clk_get'd */
+/* Note that CLKFLAGS_BOOT_ON and CLKFLAGS_CANNOT_DISABLE overrides
+ * CLKFLAGS_BOOT_OFF */
+
 struct clk {
 	struct list_head      list;
 	struct module        *owner;
@@ -79,6 +90,7 @@ struct clk {
 	struct powerdomain	*bd;
 	struct list_head	blockgating_list;
 #endif
+	unsigned int		flags;
 };
 
 /* other clocks which may be registered by board support */
-- 
1.6.3.3

[PATCH 4/4] ARM: S5PV210: Clock Framework: Flag Support for struct clk.

[MyungJoo Ham]

Added .flags property to struct clk init_clocks[] and removed
init_clocks_disabled[], which became useless by BOOT_OFF bit of .flags.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s5pv210/clock.c |  477 +++++++++++++++++++++++------------------
 1 files changed, 270 insertions(+), 207 deletions(-)

diff --git a/arch/arm/mach-s5pv210/clock.c b/arch/arm/mach-s5pv210/clock.c
index af2af5e..bafba6f 100644
--- a/arch/arm/mach-s5pv210/clock.c
+++ b/arch/arm/mach-s5pv210/clock.c
@@ -660,8 +660,51 @@ static struct clk_ops clk_hclk_imem_ops = {
 	.get_rate	= s5pv210_clk_imem_get_rate,
 };
 
-static struct clk init_clocks_disable[] = {
+static struct clk init_clocks[] = {
 	{
+		.name		= "hclk_imem",
+		.id		= -1,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 5),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+		.ops		= &clk_hclk_imem_ops,
+	}, {
+		.name		= "pdma",
+		.id		= 0,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 3),
+		.flags		= CLKFLAGS_BOOT_ON,
+	}, {
+		.name		= "pdma",
+		.id		= 1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 4),
+		.flags		= CLKFLAGS_BOOT_ON,
+	}, {
+		.name		= "mdma",
+		.id		= -1,
+		.parent		= &clk_hclk_dsys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 2),
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "dmc",
+		.id		= 0,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 0),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "dmc",
+		.id		= 1,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip0_ctrl,
+		.ctrlbit	= (1 << 1),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
 		.name		= "csis",
 		.id		= -1,
 		.parent		= &clk_pclk_dsys.clk,
@@ -669,6 +712,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 31),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "rot",
 		.id		= -1,
@@ -677,6 +721,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1<<29),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "jpeg",
 		.id		= -1,
@@ -685,6 +730,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 28),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "fimc",
 		.id		= 0,
@@ -693,6 +739,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 24),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "fimc",
 		.id		= 1,
@@ -701,6 +748,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 25),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "fimc",
 		.id		= 2,
@@ -709,6 +757,15 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 26),
 		PD_SET(cam)
 		BD_SET(img)
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "nandxl",
+		.id		= -1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip1_ctrl,
+		.ctrlbit	= (1 << 24),
+		BD_SET(memory)
+		.flags		= CLKFLAGS_BOOT_ON,
 	}, {
 		.name		= "nfcon",
 		.id		= -1,
@@ -716,6 +773,15 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1 << 28),
 		BD_SET(memory)
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "sromc",
+		.id		= -1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip1_ctrl,
+		.ctrlbit	= (1 << 26),
+		BD_SET(memory)
+		.flags		= CLKFLAGS_BOOT_ON,
 	}, {
 		.name		= "tvenc",
 		.id		= -1,
@@ -724,6 +790,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 10),
 		PD_SET(tv)
 		BD_SET(tv)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "hdmi",
 		.id		= -1,
@@ -732,6 +799,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 11),
 		PD_SET(tv)
 		BD_SET(tv)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "mixer",
 		.id		= -1,
@@ -740,6 +808,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 9),
 		PD_SET(tv)
 		BD_SET(tv)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "otg",
 		.id		= -1,
@@ -747,6 +816,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<16),
 		BD_SET(usb)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "usb-host",
 		.id		= -1,
@@ -754,6 +824,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<17),
 		BD_SET(usb)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "lcd",
 		.id		= -1,
@@ -762,6 +833,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1<<0),
 		PD_SET(lcd)
 		BD_SET(lcd)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "cfcon",
 		.id		= 0,
@@ -769,6 +841,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip1_ctrl,
 		.ctrlbit	= (1<<25),
 		BD_SET(memory)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "vp",
 		.id		= -1,
@@ -777,6 +850,7 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 8),
 		PD_SET(tv)
 		BD_SET(tv)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "dsim",
 		.id		= -1,
@@ -785,6 +859,100 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1 << 2),
 		PD_SET(lcd)
 		BD_SET(lcd)
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "tzic",
+		.id		= 0,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 28),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+		BD_SET(intc)
+	}, {
+		.name		= "tzic",
+		.id		= 1,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 29),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+		BD_SET(intc)
+	}, {
+		.name		= "tzic",
+		.id		= 2,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 30),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+		BD_SET(intc)
+	}, {
+		.name		= "tzic",
+		.id		= 3,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 31),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+		BD_SET(intc)
+	}, {
+		.name		= "vic",
+		.id		= 0,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 24),
+		BD_SET(intc)
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "vic",
+		.id		= 1,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 25),
+		BD_SET(intc)
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "vic",
+		.id		= 2,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 26),
+		BD_SET(intc)
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "vic",
+		.id		= 3,
+		.parent		= &clk_hclk_msys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 27),
+		BD_SET(intc)
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "secjtag",
+		.id		= -1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 11),
+		BD_SET(debug)
+	}, {
+		.name		= "coresight",
+		.id		= -1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 8),
+		BD_SET(debug)
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "sdm",
+		.id		= -1,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 1),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "secss",
+		.id		= -1,
+		.parent		= &clk_hclk_psys.clk,
+		.enable		= s5pv210_clk_ip2_ctrl,
+		.ctrlbit	= (1 << 0),
+		BD_SET(security)
+		.flags		= CLKFLAGS_ALWAYS_ON,
 	}, {
 		.name		= "hsmmc",
 		.id		= 0,
@@ -792,6 +960,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<16),
 		BD_SET(hsmmc)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "hsmmc",
 		.id		= 1,
@@ -799,6 +968,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<17),
 		BD_SET(hsmmc)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "hsmmc",
 		.id		= 2,
@@ -806,6 +976,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<18),
 		BD_SET(hsmmc)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "hsmmc",
 		.id		= 3,
@@ -813,6 +984,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1<<19),
 		BD_SET(hsmmc)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "tsi",
 		.id		= -1,
@@ -820,6 +992,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 20),
 		BD_SET(hsmmc)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "hostif",
 		.id		= -1,
@@ -827,6 +1000,7 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 10),
 		BD_SET(debug)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "modem",
 		.id		= -1,
@@ -834,108 +1008,168 @@ static struct clk init_clocks_disable[] = {
 		.enable		= s5pv210_clk_ip2_ctrl,
 		.ctrlbit	= (1 << 9),
 		BD_SET(debug)
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "syscon",
+		.id		= -1,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 27),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "gpio",
+		.id		= -1,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 26),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "uart",
+		.id		= 0,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 17),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "uart",
+		.id		= 1,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 18),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "uart",
+		.id		= 2,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 19),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "uart",
+		.id		= 3,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip3_ctrl,
+		.ctrlbit	= (1 << 20),
+		.flags		= CLKFLAGS_ALWAYS_ON,
 	}, {
 		.name		= "pcm",
 		.id		= 0,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 28),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "pcm",
 		.id		= 1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 29),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "pcm",
 		.id		= 2,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 30),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "systimer",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<16),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "watchdog",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<22),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "rtc",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<15),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2c",
 		.id		= 0,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<7),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2c",
 		.id		= 1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<8),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2c",
 		.id		= 2,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<9),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "spi",
 		.id		= 0,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<12),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "spi",
 		.id		= 1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<13),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "spi",
 		.id		= 2,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<14),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "timers",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<23),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "adc",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<24),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "keypad",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1<<21),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2c_hdmi_phy",
 		.id		= -1,
 		.parent		= &clk_pclk_dsys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 11),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2c_hdmi_ddc",
 		.id		= -1,
 		.parent		= &clk_pclk_dsys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 10),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2s_v50",
 		.id		= 0,
@@ -944,248 +1178,88 @@ static struct clk init_clocks_disable[] = {
 		.ctrlbit	= (1<<4),
 		/* Note that only i2s-0 is connected to pd_audio */
 		PD_SET(audio)
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2s_v32",
 		.id		= 0,
 		.parent		= &clk_p,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 5),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "i2s_v32",
 		.id		= 1,
 		.parent		= &clk_p,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 6),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "ac97",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 1),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "spdif",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip3_ctrl,
 		.ctrlbit	= (1 << 0),
+		.flags		= CLKFLAGS_BOOT_OFF,
+	}, {
+		.name		= "tzpc",
+		.id		= 0,
+		.parent		= &clk_pclk_msys.clk,
+		.enable		= s5pv210_clk_ip4_ctrl,
+		.ctrlbit	= (1 << 5),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "tzpc",
+		.id		= 1,
+		.parent		= &clk_pclk_psys.clk,
+		.enable		= s5pv210_clk_ip4_ctrl,
+		.ctrlbit	= (1 << 6),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "tzpc",
+		.id		= 2,
+		.enable		= s5pv210_clk_ip4_ctrl,
+		.ctrlbit	= (1 << 7),
+		.flags		= CLKFLAGS_ALWAYS_ON,
+	}, {
+		.name		= "tzpc",
+		.id		= 3,
+		.enable		= s5pv210_clk_ip4_ctrl,
+		.ctrlbit	= (1 << 8),
+		.flags		= CLKFLAGS_ALWAYS_ON,
 	}, {
 		.name		= "seckey",
 		.id		= -1,
 		.enable		= s5pv210_clk_ip4_ctrl,
 		.ctrlbit	= (1 << 3),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "iem_apc",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip4_ctrl,
 		.ctrlbit	= (1 << 2),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "iem_iec",
 		.id		= -1,
 		.parent		= &clk_pclk_psys.clk,
 		.enable		= s5pv210_clk_ip4_ctrl,
 		.ctrlbit	= (1 << 1),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	}, {
 		.name		= "chip_id",
 		.id		= -1,
 		.parent		= &clk_hclk_psys.clk,
 		.enable		= s5pv210_clk_ip4_ctrl,
 		.ctrlbit	= (1 << 0),
-	},
-};
-
-static struct clk init_clocks[] = {
-	{
-		.name		= "hclk_imem",
-		.id		= -1,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 5),
-		.ops		= &clk_hclk_imem_ops,
-	}, {
-		.name		= "pdma",
-		.id		= 0,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 3),
-	}, {
-		.name		= "pdma",
-		.id		= 1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 4),
-	}, {
-		.name		= "mdma",
-		.id		= -1,
-		.parent		= &clk_hclk_dsys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 2),
-	}, {
-		.name		= "dmc",
-		.id		= 0,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 0),
-	}, {
-		.name		= "dmc",
-		.id		= 1,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip0_ctrl,
-		.ctrlbit	= (1 << 1),
-	}, {
-		.name		= "nandxl",
-		.id		= -1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip1_ctrl,
-		.ctrlbit	= (1 << 24),
-		BD_SET(memory)
-	}, {
-		.name		= "sromc",
-		.id		= -1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip1_ctrl,
-		.ctrlbit	= (1 << 26),
-		BD_SET(memory)
-	}, {
-		.name		= "tzic",
-		.id		= 0,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 28),
-		BD_SET(intc)
-	}, {
-		.name		= "tzic",
-		.id		= 1,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 29),
-		BD_SET(intc)
-	}, {
-		.name		= "tzic",
-		.id		= 2,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 30),
-		BD_SET(intc)
-	}, {
-		.name		= "tzic",
-		.id		= 3,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 31),
-		BD_SET(intc)
-	}, {
-		.name		= "vic",
-		.id		= 0,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 24),
-		BD_SET(intc)
-	}, {
-		.name		= "vic",
-		.id		= 1,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 25),
-		BD_SET(intc)
-	}, {
-		.name		= "vic",
-		.id		= 2,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 26),
-		BD_SET(intc)
-	}, {
-		.name		= "vic",
-		.id		= 3,
-		.parent		= &clk_hclk_msys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 27),
-		BD_SET(intc)
-	}, {
-		.name		= "secjtag",
-		.id		= -1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 11),
-		BD_SET(debug)
-	}, {
-		.name		= "coresight",
-		.id		= -1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 8),
-		BD_SET(debug)
-	}, {
-		.name		= "sdm",
-		.id		= -1,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 1),
-	}, {
-		.name		= "secss",
-		.id		= -1,
-		.parent		= &clk_hclk_psys.clk,
-		.enable		= s5pv210_clk_ip2_ctrl,
-		.ctrlbit	= (1 << 0),
-		BD_SET(security)
-	}, {
-		.name		= "syscon",
-		.id		= -1,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 27),
-	}, {
-		.name		= "gpio",
-		.id		= -1,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 26),
-	}, {
-		.name		= "uart",
-		.id		= 0,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 17),
-	}, {
-		.name		= "uart",
-		.id		= 1,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 18),
-	}, {
-		.name		= "uart",
-		.id		= 2,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 19),
-	}, {
-		.name		= "uart",
-		.id		= 3,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip3_ctrl,
-		.ctrlbit	= (1 << 20),
-	}, {
-		.name		= "tzpc",
-		.id		= 0,
-		.parent		= &clk_pclk_msys.clk,
-		.enable		= s5pv210_clk_ip4_ctrl,
-		.ctrlbit	= (1 << 5),
-	}, {
-		.name		= "tzpc",
-		.id		= 1,
-		.parent		= &clk_pclk_psys.clk,
-		.enable		= s5pv210_clk_ip4_ctrl,
-		.ctrlbit	= (1 << 6),
-	}, {
-		.name		= "tzpc",
-		.id		= 2,
-		.enable		= s5pv210_clk_ip4_ctrl,
-		.ctrlbit	= (1 << 7),
-	}, {
-		.name		= "tzpc",
-		.id		= 3,
-		.enable		= s5pv210_clk_ip4_ctrl,
-		.ctrlbit	= (1 << 8),
+		.flags		= CLKFLAGS_BOOT_OFF,
 	},
 };
 
@@ -1757,7 +1831,6 @@ static struct clk *clks[] __initdata = {
 
 void __init s5pv210_register_clocks(void)
 {
-	struct clk *clkp;
 	int ret;
 	int ptr;
 
@@ -1771,16 +1844,6 @@ void __init s5pv210_register_clocks(void)
 	s3c_register_clksrc(clksrcs, ARRAY_SIZE(clksrcs));
 	s3c_register_clocks(init_clocks, ARRAY_SIZE(init_clocks));
 
-	clkp = init_clocks_disable;
-	for (ptr = 0; ptr < ARRAY_SIZE(init_clocks_disable); ptr++, clkp++) {
-		ret = s3c24xx_register_clock(clkp);
-		if (ret < 0) {
-			printk(KERN_ERR "Failed to register clock %s (%d)\n",
-			       clkp->name, ret);
-		}
-		(clkp->enable)(clkp, 0);
-	}
-
 #ifdef CONFIG_S5PV210_POWERDOMAIN
 	/* According to the state of the members, turn on/off power domains
 	 * and blocks. */
-- 
1.6.3.3

[PATCH 3/4] ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.

[Russell King - ARM Linux]

On Mon, Jul 19, 2010 at 05:30:35PM +0900, MyungJoo Ham wrote:
> +#ifdef CONFIG_SAMSUNG_POWERDOMAIN
> +		if ((clk->flags | CLKFLAGS_BOOT_ON) &&

This is always true if CLKFLAGS_BOOT_ON is non-zero.

> +				!(clk->flags | CLKFLAGS_CANNOT_DISABLE)) {

This is always false if CLKFLAGS_CANNOT_DISABLE is non-zero.

> +		if (clk->enable && !(clk->flags | CLKFLAGS_CANNOT_DISABLE))

Always false...

[PATCH 2/4] ARM: S5PV210: Powerdomain/Clock-gating Support

[Russell King - ARM Linux]

On Mon, Jul 19, 2010 at 05:30:34PM +0900, MyungJoo Ham wrote:
> +static int powerdomain_set(struct powerdomain *pd, int enable)
> +{
> +	unsigned long ctrlbit;
> +	void __iomem *reg;
> +	void __iomem *stable_reg;
> +	unsigned long reg_dat;
> +
> +	if (pd == NULL)
> +		return -EINVAL;

If someone calls this function with a NULL pd argument, is it better
to ignore it, or better to use WARN_ON and get a backtrace so it can
be fixed?  I don't see much reason for this function to ever be called
with a NULL powerdomain argument.

> +
> +	ctrlbit = pd->pd_ctrlbit;
> +	reg = (void __iomem *)pd->pd_reg;
> +	stable_reg = (void __iomem *)pd->pd_stable_reg;

Why not ensure that the element in the structure is correctly typed to
start with?

It's good practice to avoid casts whenever-possible - they hide bugs.

[PATCH 3/4] ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.

[Maurus Cuelenaere]

 Op 19-07-10 10:30, MyungJoo Ham schreef:
> Add a .flag property to the struct clk. The flag can have the following
> information:
>
> 	The clock is enabled at the boot time.
> 	The clock cannot be disabled.
> 	The clock is enabled at the boot time and cannot be disabled.
> 	The clock is disabled at the boot time. Note that both
> CLKFLAGS_BOOT_ON and CLKFLAGS_CANNOT_DISABLE can override
> CLKFLAGS_BOOT_OFF.
> 	Please stop using this clock. When a DEPRECATED clock is
> clk_get'd, clk_get function will printk a warning message.
>
> The previous patch related with powerdomain / blcok-gating control
> requires this patch for the stability related with clocks that are
> turned on at the boot time.
>
> Note that clocks without both BOOT_ON and BOOT_OFF keep the previous
> states; however, powerdomain / block-gating controls do NOT have any
> information about the states of such clocks, which in turn, may incur
> instable kernel behaviors. For example, a powerdomain may be turned off
> while a clock of the domain is still being used. With the flag support
> feature, we highly recommend to define .flag fields fully for clocks
> related to powerdomain and block-gating. Clocks not connected to
> powerdomain and block-gating are ok without flag field.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/clock.c              |   70 ++++++++++++++++++++++++++-
>  arch/arm/plat-samsung/include/plat/clock.h |   12 +++++
>  2 files changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c
> index f8a30f7..9ff9d63 100644
> --- a/arch/arm/plat-samsung/clock.c
> +++ b/arch/arm/plat-samsung/clock.c
> @@ -103,6 +103,14 @@ struct clk *clk_get(struct device *dev, const char *id)
>  	}
>  
>  	spin_unlock(&clocks_lock);
> +
> +	if (!IS_ERR(clk) && clk)
> +		if (clk->flags & CLKFLAGS_DEPRECATED)
> +			printk(KERN_WARNING "[%s:%d] clk %s:%d is deprecated. "
> +					"Please reconsider using it.\n",
> +					__FILE__, __LINE__, clk->name,
> +					clk->id);

What's the use of __FILE__ and __LINE__? If you wanted to print the caller you
could do something like pr_warn("[%pS]: ...", __builtin_return_address(0), ...).

Also, pr_warn(...)

> +
>  	return clk;
>  }
>  
> @@ -169,7 +177,25 @@ void clk_disable(struct clk *clk)
>  	spin_lock(&clocks_lock);
>  
>  	if ((--clk->usage) == 0) {
> -		(clk->enable)(clk, 0);
> +#ifdef CONFIG_SAMSUNG_POWERDOMAIN
> +		if ((clk->flags | CLKFLAGS_BOOT_ON) &&
> +				!(clk->flags | CLKFLAGS_CANNOT_DISABLE)) {

Russel already mentioned this, did you wanted to do (clk->flags & CLKFLAGS_*)?

> +			/* BOOT_ON became NO EFFECT. Let PD/BD be able
> +			 * to turn themselves off */
> +
> +			if (clk->pd)
> +				clk->pd->num_clks_boot_on--;
> +#ifdef CONFIG_SAMSUNG_BLOCKGATING
> +			if (clk->bd)
> +				clk->bd->num_clks_boot_on--;
> +#endif
> +			clk->flags &= ~CLKFLAGS_BOOT_ON;
> +		}
> +#endif
> +
> +		if (clk->enable && !(clk->flags | CLKFLAGS_CANNOT_DISABLE))
> +			(clk->enable)(clk, 0);
> +
>  #ifdef CONFIG_SAMSUNG_POWERDOMAIN
>  		if (clk->pd) {
>  			if (clk->pd->ref_count == 1 &&
> @@ -193,7 +219,9 @@ void clk_disable(struct clk *clk)
>  	}
>  
>  	spin_unlock(&clocks_lock);
> -	clk_disable(clk->parent);
> +
> +	if (!(clk->flags | CLKFLAGS_CANNOT_DISABLE))
> +		clk_disable(clk->parent);
>  }
>  
>  
> @@ -370,6 +398,13 @@ struct clk s3c24xx_uclk = {
>   */
>  int s3c24xx_register_clock(struct clk *clk)
>  {
> +	int ret = 0;
> +
> +	if (clk == NULL)
> +		return -EINVAL;
> +	if (clk->name == NULL)
> +		return -EINVAL;
> +
>  	if (clk->enable == NULL)
>  		clk->enable = clk_null_enable;
>  
> @@ -382,6 +417,21 @@ int s3c24xx_register_clock(struct clk *clk)
>  	list_add(&clk->list, &clocks);
>  	spin_unlock(&clocks_lock);
>  
> +	if (clk->flags & CLKFLAGS_BOOT_ON) {
> +		/* Use clk_enable, not clk->enable as clk's parent is
> +		 * also required to be turned on */
> +		clk_enable(clk);
> +
> +#ifdef CONFIG_SAMSUNG_POWERDOMAIN
> +		if (clk->pd)
> +			clk->pd->num_clks_boot_on++;
> +#endif
> +#ifdef CONFIG_SAMSUNG_BLOCKGATING
> +		if (clk->bd)
> +			clk->bd->num_clks_boot_on++;
> +#endif
> +	}
> +
>  #ifdef CONFIG_SAMSUNG_POWERDOMAIN
>  	if (clk->pd) {
>  		spin_lock(&clocks_lock);
> @@ -397,7 +447,21 @@ int s3c24xx_register_clock(struct clk *clk)
>  	}
>  #endif
>  
> -	return 0;
> +	if (clk->flags & CLKFLAGS_BOOT_OFF) {
> +		if ((clk->flags & CLKFLAGS_BOOT_ON) ||
> +				(clk->flags & CLKFLAGS_CANNOT_DISABLE)) {
> +			printk(KERN_WARNING "[%s:%d] clk %s:%d has incompatible"
> +					" initilization flags: %x.\n",
> +					__FILE__, __LINE__, clk->name, clk->id,
> +					clk->flags);

Same comment, also s/initilization/initialization/.

> +			ret = -EINVAL;
> +		} else
> +			/* Do NOT use clk_disable(clk) because clk_disable
> +			 * may disable clk's parent */
> +			ret = (clk->enable)(clk, 0);
> +	}
> +
> +	return ret;
>  }
>  
>  /**
> diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-samsung/include/plat/clock.h
> index bf851e1..0f7d556 100644
> --- a/arch/arm/plat-samsung/include/plat/clock.h
> +++ b/arch/arm/plat-samsung/include/plat/clock.h
> @@ -59,6 +59,17 @@ struct clk_ops {
>  	int		    (*set_parent)(struct clk *c, struct clk *parent);
>  };
>  
> +/* Note that when BOOT_ON and OFF are both not set, the kernel
> + * keeps the state initialized by the boot-loader or cpu.
> + */
> +#define CLKFLAGS_BOOT_ON	(0x1)
> +#define CLKFLAGS_CANNOT_DISABLE	(0x2)
> +#define CLKFLAGS_ALWAYS_ON	(CLKFLAGS_BOOT_ON | CLKFLAGS_CANNOT_DISABLE)
> +#define CLKFLAGS_BOOT_OFF	(0x4) /* Force off when boot */
> +#define CLKFLAGS_DEPRECATED	(0x8) /* Warn when clk_get'd */
> +/* Note that CLKFLAGS_BOOT_ON and CLKFLAGS_CANNOT_DISABLE overrides
> + * CLKFLAGS_BOOT_OFF */
> +
>  struct clk {
>  	struct list_head      list;
>  	struct module        *owner;
> @@ -79,6 +90,7 @@ struct clk {
>  	struct powerdomain	*bd;
>  	struct list_head	blockgating_list;
>  #endif
> +	unsigned int		flags;
>  };
>  
>  /* other clocks which may be registered by board support */


-- 
Maurus Cuelenaere

[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> This patches add support for powerdomain, block-gating, and flags in
struct clk.
> 
> Blockgating re-uses powerdomain support scheme and depends on powerdomain
> support.
> 
> Flags support is independent from powerdomain; however, powerdomain
support
> is NOT stable without flags support. Without flags support, powerdomain
may lock
> up the system with some conditions although they are rare. Thus, when
> powerdomain or block-gating is used, flags support should be turned on for
the
> system stability. (and that's why I'm sending flags support with
> powerdomain/block-gating support).
> 
> Although powerdomain support is requred for blockgating, blockgating is
NOT
> required for powerdomain. Besides, powerdomain support is observed to
reduce
> power consumption significantly (with 2.6.29 kernel); however, blockgating
support
> didn't show any significant improvement.
> 
> 
> MyungJoo Ham (4):
>   ARM: SAMSUNG SoC: Powerdomain/Block-gating Support
>   ARM: S5PV210: Powerdomain/Clock-gating Support
>   ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.
>   ARM: S5PV210: Clock Framework: Flag Support for struct clk.
> 
>  arch/arm/mach-s5pv210/Kconfig              |   17 +
>  arch/arm/mach-s5pv210/clock.c              |  906
> ++++++++++++++++++++++------
>  arch/arm/plat-samsung/Kconfig              |   19 +
>  arch/arm/plat-samsung/clock.c              |  146 +++++-
>  arch/arm/plat-samsung/include/plat/clock.h |   44 ++
>  5 files changed, 935 insertions(+), 197 deletions(-)

Already we discussed about power domain...
And actually your code is similar with my member's 2.6.29 powerdomain
scheme.
So Ben's code review about that is available to your this patch.

Following is from Ben Dooks note.

===

Powerdomain control notes
=========================

Copyright 2010 Simtec Electronics
Ben Dooks <ben@simtec.co.uk>

Code Review
-----------

The current implementation makes a number of #ifdef based additions to
the clock.c which currently lies in plat-s3c/clock.c (but to be moved
to plat-samsung/clock.c). The following are the observations from reviewing
the code as presented:

1) The use of #ifdef is not recommended, as noted before in reviews it
   makes the code hard to read, allows bugs to creep in due to code
   either being skipped, and problems when we get to the stage several
   conflicting configurations could live in the kernel.

2) The code is changing the functionality of the clk_enable() and the
   clk_disable() calls, which is bad for the following reasons:

   A) Anyone reading or modifying a driver using this API may not see
      what is going on underneath.

   B) It ties the clock to the powerdomain, if any of the current drivers
      wish to temporarily stop the clock to reduce the dynamic power
      consumption they cannot (see part A, if all drivers are resting
      in the power domain the whole domain may shutdown with a resulting
      cost to the work resumption).

   C) It ties the drivers to the specific clock implementation and thus
      restricts future use of standard drivers on future devices.

   D) It ties policy into the kernel, as it forces the power domains to
      shut down if the driver is not in use. The developer or end user
      may want to change this depending on either a device wide or usage
      case. Policy decisions are best left out of the kernel.


Part (1) is a definite barrier to merging, the code whilst functional is
both ugly and the use of #ifdefs like that are contrary to a number of
developers beliefs.

The second part is also a stumbling block, as it is likely to be commented
upon by a number of interested parties if it did come up for review. I am
certainly not happy to see this sort of invasive change to the clock system
merged. I expect others would also raise objections.

...

===

So need to another approach for it.

Thanks.

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

[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[MyungJoo Ham]

On Tue, Jul 20, 2010 at 10:04 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> MyungJoo Ham wrote:
>>
>> This patches add support for powerdomain, block-gating, and flags in
> struct clk.
>>
>> Blockgating re-uses powerdomain support scheme and depends on powerdomain
>> support.
>>
>> Flags support is independent from powerdomain; however, powerdomain
> support
>> is NOT stable without flags support. Without flags support, powerdomain
> may lock
>> up the system with some conditions although they are rare. Thus, when
>> powerdomain or block-gating is used, flags support should be turned on for
> the
>> system stability. (and that's why I'm sending flags support with
>> powerdomain/block-gating support).
>>
>> Although powerdomain support is requred for blockgating, blockgating is
> NOT
>> required for powerdomain. Besides, powerdomain support is observed to
> reduce
>> power consumption significantly (with 2.6.29 kernel); however, blockgating
> support
>> didn't show any significant improvement.
>>
>>
>> MyungJoo Ham (4):
>> ? ARM: SAMSUNG SoC: Powerdomain/Block-gating Support
>> ? ARM: S5PV210: Powerdomain/Clock-gating Support
>> ? ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.
>> ? ARM: S5PV210: Clock Framework: Flag Support for struct clk.
>>
>> ?arch/arm/mach-s5pv210/Kconfig ? ? ? ? ? ? ?| ? 17 +
>> ?arch/arm/mach-s5pv210/clock.c ? ? ? ? ? ? ?| ?906
>> ++++++++++++++++++++++------
>> ?arch/arm/plat-samsung/Kconfig ? ? ? ? ? ? ?| ? 19 +
>> ?arch/arm/plat-samsung/clock.c ? ? ? ? ? ? ?| ?146 +++++-
>> ?arch/arm/plat-samsung/include/plat/clock.h | ? 44 ++
>> ?5 files changed, 935 insertions(+), 197 deletions(-)
>
> Already we discussed about power domain...
> And actually your code is similar with my member's 2.6.29 powerdomain
> scheme.
> So Ben's code review about that is available to your this patch.
>
> Following is from Ben Dooks note.
>
> ===
>
> Powerdomain control notes
> =========================
>
> Copyright 2010 Simtec Electronics
> Ben Dooks <ben@simtec.co.uk>
>
> Code Review
> -----------
>
> The current implementation makes a number of #ifdef based additions to
> the clock.c which currently lies in plat-s3c/clock.c (but to be moved
> to plat-samsung/clock.c). The following are the observations from reviewing
> the code as presented:
>
> 1) The use of #ifdef is not recommended, as noted before in reviews it
> ? makes the code hard to read, allows bugs to creep in due to code
> ? either being skipped, and problems when we get to the stage several
> ? conflicting configurations could live in the kernel.

I've been using #ifdef's for enabling/disabling powerdomain and
blockgating control (especially for machines that do not have support
for powerdomain/blockgating). However, it seems to be reasonable that
the #ifdef's are not needed and to be removed.

>
> 2) The code is changing the functionality of the clk_enable() and the
> ? clk_disable() calls, which is bad for the following reasons:
>
> ? A) Anyone reading or modifying a driver using this API may not see
> ? ? ?what is going on underneath.
>
> ? B) It ties the clock to the powerdomain, if any of the current drivers
> ? ? ?wish to temporarily stop the clock to reduce the dynamic power
> ? ? ?consumption they cannot (see part A, if all drivers are resting
> ? ? ?in the power domain the whole domain may shutdown with a resulting
> ? ? ?cost to the work resumption).
>
> ? C) It ties the drivers to the specific clock implementation and thus
> ? ? ?restricts future use of standard drivers on future devices.
>
> ? D) It ties policy into the kernel, as it forces the power domains to
> ? ? ?shut down if the driver is not in use. The developer or end user
> ? ? ?may want to change this depending on either a device wide or usage
> ? ? ?case. Policy decisions are best left out of the kernel.
>

Um... yes, putting powerdomain/blockgating code at clk_* code is too
invasive to the clk_* code. And it'd be more problematic if we move on
to the common struct clk.

I'll implement powerdomain/blockgating control code
 at arch/arm/mach-s5pv210/clock.c:s5pv210_clk_*_ctrl.

For B) and D), we may need to leave an option not to control
powerdomain. How about leaving an option at sysfs if not using
#ifdef's?


>
> Part (1) is a definite barrier to merging, the code whilst functional is
> both ugly and the use of #ifdefs like that are contrary to a number of
> developers beliefs.
>
> The second part is also a stumbling block, as it is likely to be commented
> upon by a number of interested parties if it did come up for review. I am
> certainly not happy to see this sort of invasive change to the clock system
> merged. I expect others would also raise objections.
>
> ...
>
> ===
>
> So need to another approach for it.
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[Ben Dooks]

On 07/20/10 04:05, MyungJoo Ham wrote:
> On Tue, Jul 20, 2010 at 10:04 AM, Kukjin Kim<kgene.kim@samsung.com>  wrote:
>> MyungJoo Ham wrote:
>>>
>>> This patches add support for powerdomain, block-gating, and flags in
>> struct clk.
>>>
>>> Blockgating re-uses powerdomain support scheme and depends on powerdomain
>>> support.
>>>
>>> Flags support is independent from powerdomain; however, powerdomain
>> support
>>> is NOT stable without flags support. Without flags support, powerdomain
>> may lock
>>> up the system with some conditions although they are rare. Thus, when
>>> powerdomain or block-gating is used, flags support should be turned on for
>> the
>>> system stability. (and that's why I'm sending flags support with
>>> powerdomain/block-gating support).
>>>
>>> Although powerdomain support is requred for blockgating, blockgating is
>> NOT
>>> required for powerdomain. Besides, powerdomain support is observed to
>> reduce
>>> power consumption significantly (with 2.6.29 kernel); however, blockgating
>> support
>>> didn't show any significant improvement.
>>>
>>>
>>> MyungJoo Ham (4):
>>>    ARM: SAMSUNG SoC: Powerdomain/Block-gating Support
>>>    ARM: S5PV210: Powerdomain/Clock-gating Support
>>>    ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.
>>>    ARM: S5PV210: Clock Framework: Flag Support for struct clk.
>>>
>>>   arch/arm/mach-s5pv210/Kconfig              |   17 +
>>>   arch/arm/mach-s5pv210/clock.c              |  906
>>> ++++++++++++++++++++++------
>>>   arch/arm/plat-samsung/Kconfig              |   19 +
>>>   arch/arm/plat-samsung/clock.c              |  146 +++++-
>>>   arch/arm/plat-samsung/include/plat/clock.h |   44 ++
>>>   5 files changed, 935 insertions(+), 197 deletions(-)
>>
>> Already we discussed about power domain...
>> And actually your code is similar with my member's 2.6.29 powerdomain
>> scheme.
>> So Ben's code review about that is available to your this patch.
>>
>> Following is from Ben Dooks note.
>>
>> ===

My powers of psychic prediction are legendary now :0)

>> Powerdomain control notes
>> =========================
>>
>> Copyright 2010 Simtec Electronics
>> Ben Dooks<ben@simtec.co.uk>
>>
>> Code Review
>> -----------
>>
>> The current implementation makes a number of #ifdef based additions to
>> the clock.c which currently lies in plat-s3c/clock.c (but to be moved
>> to plat-samsung/clock.c). The following are the observations from reviewing
>> the code as presented:
>>
>> 1) The use of #ifdef is not recommended, as noted before in reviews it
>>    makes the code hard to read, allows bugs to creep in due to code
>>    either being skipped, and problems when we get to the stage several
>>    conflicting configurations could live in the kernel.
>
> I've been using #ifdef's for enabling/disabling powerdomain and
> blockgating control (especially for machines that do not have support
> for powerdomain/blockgating). However, it seems to be reasonable that
> the #ifdef's are not needed and to be removed.
>
>>
>> 2) The code is changing the functionality of the clk_enable() and the
>>    clk_disable() calls, which is bad for the following reasons:
>>
>>    A) Anyone reading or modifying a driver using this API may not see
>>       what is going on underneath.
>>
>>    B) It ties the clock to the powerdomain, if any of the current drivers
>>       wish to temporarily stop the clock to reduce the dynamic power
>>       consumption they cannot (see part A, if all drivers are resting
>>       in the power domain the whole domain may shutdown with a resulting
>>       cost to the work resumption).
>>
>>    C) It ties the drivers to the specific clock implementation and thus
>>       restricts future use of standard drivers on future devices.
>>
>>    D) It ties policy into the kernel, as it forces the power domains to
>>       shut down if the driver is not in use. The developer or end user
>>       may want to change this depending on either a device wide or usage
>>       case. Policy decisions are best left out of the kernel.
>>
>
> Um... yes, putting powerdomain/blockgating code at clk_* code is too
> invasive to the clk_* code. And it'd be more problematic if we move on
> to the common struct clk.

As a note, you could add virtual clocks, one per power domain which
have their enable function point into the power domain code. This
should be called each time the clock is enabled/disabled and thus
allows easier integration that the current method. It should also
avoid a pile of #ifdefs.

> I'll implement powerdomain/blockgating control code
>   at arch/arm/mach-s5pv210/clock.c:s5pv210_clk_*_ctrl.
>
> For B) and D), we may need to leave an option not to control
> powerdomain. How about leaving an option at sysfs if not using
> #ifdef's?

I'd like to see something more generic, however this approach has
its own merits. The plus side is it'll save power, the down sides
are as follows:

1) Drivers are now losing their clock _and_ power, really the driver
    should be quiesced by runtime-pm otherwise we end up relying on
    the lower level behaviour of the clock system (undocumented) and
    having drivers have to restore settings when they do clk_enable()

    This is fine for Samsung specific drivers, but is going to be really
    bad for everyone else.

2) Some devices, such as the SDHCI need clocks after the operation has
    finished to allow cards to complete

3) How much time is spent taking the power domains up/down? Is this time
    we could be usefully using to actually do work for the user?

The big one here is #1.

I would much prefer to see some framework which controls when devices
are going to be scheduled so that they can be put to sleep when it is
unlikely they're going to be used, and the system's performance can be
controlled (ie, if the device is in hard-use, such as decoding video
it is probably unlikely we'll want to shutdown the data source at-all)
another example would be if the device is idling, it would be very
permissible to shut things down when they are not in use as the system
does not need a lot of performance at that point.

I've suggested hooking power-domain support into the regulator framework
but Mark's pointed out some problems there. it may be worth using the
regulator framework as a low-level system so that regulators can be
chained off (ie, control external ldos/switches/etc).

I'd be happier to see something where the device drivers use runtime pm
(possibly with the regulator framework) to allow the user to stop the
system. We could also look at device in-use doing auto runtime-pm to
say open => resume, and close => suspend.

>> Part (1) is a definite barrier to merging, the code whilst functional is
>> both ugly and the use of #ifdefs like that are contrary to a number of
>> developers beliefs.
>>
>> The second part is also a stumbling block, as it is likely to be commented
>> upon by a number of interested parties if it did come up for review. I am
>> certainly not happy to see this sort of invasive change to the clock system
>> merged. I expect others would also raise objections.
>>
>> ...
>>
>> ===
>>
>> So need to another approach for it.

It certainly needs a pile of work before we can think about it.

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


-- 
Ben Dooks, Design & Software Engineer, Simtec Electronics

http://www.simtec.co.uk/

[PATCH 0/4] ARM: S5PV210: Clock Framework: powerdomain, block-gating, flags

[MyungJoo Ham]

On Wed, Jul 21, 2010 at 9:33 AM, Ben Dooks <ben@simtec.co.uk> wrote:
> On 07/20/10 04:05, MyungJoo Ham wrote:
>>
>> On Tue, Jul 20, 2010 at 10:04 AM, Kukjin Kim<kgene.kim@samsung.com>
>> ?wrote:
>>>
>>> MyungJoo Ham wrote:
>>>>
>>>> This patches add support for powerdomain, block-gating, and flags in
>>>
>>> struct clk.
>>>>
>>>> Blockgating re-uses powerdomain support scheme and depends on
>>>> powerdomain
>>>> support.
>>>>
>>>> Flags support is independent from powerdomain; however, powerdomain
>>>
>>> support
>>>>
>>>> is NOT stable without flags support. Without flags support, powerdomain
>>>
>>> may lock
>>>>
>>>> up the system with some conditions although they are rare. Thus, when
>>>> powerdomain or block-gating is used, flags support should be turned on
>>>> for
>>>
>>> the
>>>>
>>>> system stability. (and that's why I'm sending flags support with
>>>> powerdomain/block-gating support).
>>>>
>>>> Although powerdomain support is requred for blockgating, blockgating is
>>>
>>> NOT
>>>>
>>>> required for powerdomain. Besides, powerdomain support is observed to
>>>
>>> reduce
>>>>
>>>> power consumption significantly (with 2.6.29 kernel); however,
>>>> blockgating
>>>
>>> support
>>>>
>>>> didn't show any significant improvement.
>>>>
>>>>
>>>> MyungJoo Ham (4):
>>>> ? ARM: SAMSUNG SoC: Powerdomain/Block-gating Support
>>>> ? ARM: S5PV210: Powerdomain/Clock-gating Support
>>>> ? ARM: SAMSUNG SoC: Clock Framework: Flag Support for struct clk.
>>>> ? ARM: S5PV210: Clock Framework: Flag Support for struct clk.
>>>>
>>>> ?arch/arm/mach-s5pv210/Kconfig ? ? ? ? ? ? ?| ? 17 +
>>>> ?arch/arm/mach-s5pv210/clock.c ? ? ? ? ? ? ?| ?906
>>>> ++++++++++++++++++++++------
>>>> ?arch/arm/plat-samsung/Kconfig ? ? ? ? ? ? ?| ? 19 +
>>>> ?arch/arm/plat-samsung/clock.c ? ? ? ? ? ? ?| ?146 +++++-
>>>> ?arch/arm/plat-samsung/include/plat/clock.h | ? 44 ++
>>>> ?5 files changed, 935 insertions(+), 197 deletions(-)
>>>
>>> Already we discussed about power domain...
>>> And actually your code is similar with my member's 2.6.29 powerdomain
>>> scheme.
>>> So Ben's code review about that is available to your this patch.
>>>
>>> Following is from Ben Dooks note.
>>>
>>> ===
>
> My powers of psychic prediction are legendary now :0)

:D

>
>>> Powerdomain control notes
>>> =========================
>>>
>>> Copyright 2010 Simtec Electronics
>>> Ben Dooks<ben@simtec.co.uk>
>>>
>>> Code Review
>>> -----------
>>>
>>> The current implementation makes a number of #ifdef based additions to
>>> the clock.c which currently lies in plat-s3c/clock.c (but to be moved
>>> to plat-samsung/clock.c). The following are the observations from
>>> reviewing
>>> the code as presented:
>>>
>>> 1) The use of #ifdef is not recommended, as noted before in reviews it
>>> ? makes the code hard to read, allows bugs to creep in due to code
>>> ? either being skipped, and problems when we get to the stage several
>>> ? conflicting configurations could live in the kernel.
>>
>> I've been using #ifdef's for enabling/disabling powerdomain and
>> blockgating control (especially for machines that do not have support
>> for powerdomain/blockgating). However, it seems to be reasonable that
>> the #ifdef's are not needed and to be removed.
>>
>>>
>>> 2) The code is changing the functionality of the clk_enable() and the
>>> ? clk_disable() calls, which is bad for the following reasons:
>>>
>>> ? A) Anyone reading or modifying a driver using this API may not see
>>> ? ? ?what is going on underneath.
>>>
>>> ? B) It ties the clock to the powerdomain, if any of the current drivers
>>> ? ? ?wish to temporarily stop the clock to reduce the dynamic power
>>> ? ? ?consumption they cannot (see part A, if all drivers are resting
>>> ? ? ?in the power domain the whole domain may shutdown with a resulting
>>> ? ? ?cost to the work resumption).
>>>
>>> ? C) It ties the drivers to the specific clock implementation and thus
>>> ? ? ?restricts future use of standard drivers on future devices.
>>>
>>> ? D) It ties policy into the kernel, as it forces the power domains to
>>> ? ? ?shut down if the driver is not in use. The developer or end user
>>> ? ? ?may want to change this depending on either a device wide or usage
>>> ? ? ?case. Policy decisions are best left out of the kernel.
>>>
>>
>> Um... yes, putting powerdomain/blockgating code at clk_* code is too
>> invasive to the clk_* code. And it'd be more problematic if we move on
>> to the common struct clk.
>
> As a note, you could add virtual clocks, one per power domain which
> have their enable function point into the power domain code. This
> should be called each time the clock is enabled/disabled and thus
> allows easier integration that the current method. It should also
> avoid a pile of #ifdefs.

We've been using so-called virtual clocks for power domains that does
not have corresponding clocks (i.e., irom in S5PV210).

Because there are multiple clocks ties to a powerdomain and a
powerdomain should be turned on when one of them is turned on, using
powerdomain virtual clock as a parent can enable powerdomain feature.
However, unless we allow multiple parents per clock, this forces
clocks to lose their original parents; so, it's NO. The powerdomain
implementation in this revision of patch tried to add another "parent"
property at struct clk. In the next revision (which is under testing),
we separate powerdomain/blockgating feature from struct clk and do it
at mach-s5pv210/clock.c independently from platform's clk, which do
not have any #ifdefs or modifications in plat-*/*

>
>> I'll implement powerdomain/blockgating control code
>> ?at arch/arm/mach-s5pv210/clock.c:s5pv210_clk_*_ctrl.
>>
>> For B) and D), we may need to leave an option not to control
>> powerdomain. How about leaving an option at sysfs if not using
>> #ifdef's?
>
> I'd like to see something more generic, however this approach has
> its own merits. The plus side is it'll save power, the down sides
> are as follows:
>
> 1) Drivers are now losing their clock _and_ power, really the driver
> ? should be quiesced by runtime-pm otherwise we end up relying on
> ? the lower level behaviour of the clock system (undocumented) and
> ? having drivers have to restore settings when they do clk_enable()
>
> ? This is fine for Samsung specific drivers, but is going to be really
> ? bad for everyone else.

I think this issue is somewhat mitigated by implementing this feature
only at mach-s5pv210/*.

Besides, in order to avoid saving/restoring settings when drivers do
clk_enable/clk_disable, I've defined struct register_save *normal_ff,
which saves non-retention registers, which lose values when a
powerdomain is off. (well.. the list of those registers is not filled,
yet.) Thus, drivers do not need to do so as long as we fill these
lists out. Doing so allows us to do powerdomain control transparently
from drivers.

However, most drivers related with powerdomain seemed to be ok to lose
register values after clk_disable is called except for the audio
driver so far. So I'm thinking about saving those registers
selectively in S5PV210 although this breaks the transparency.

>
> 2) Some devices, such as the SDHCI need clocks after the operation has
> ? finished to allow cards to complete

If SDHCI needs clocks after the operation, isn't it holding its clk as
enabled and delaying clk_disable()? Does SDHCI call clk_disable()
while its clock is required to be left turned on? For devices required
to be "always on", there is the "ALWAYS_ON" flag. (and the powerdomain
control recognizes it.)

>
> 3) How much time is spent taking the power domains up/down? Is this time
> ? we could be usefully using to actually do work for the user?

When we counted the wait-for-stabilization loop iteration (e.g., do {
count++; } while(not stable);) the count value was somewhere around 12
~ 20.
When it does not need to access powerdomain (not connected with a
powerdomain or the powerdomain is already at the wanted state), it
skips accessing it.

>
> The big one here is #1.
>
> I would much prefer to see some framework which controls when devices
> are going to be scheduled so that they can be put to sleep when it is
> unlikely they're going to be used, and the system's performance can be
> controlled (ie, if the device is in hard-use, such as decoding video
> it is probably unlikely we'll want to shutdown the data source at-all)
> another example would be if the device is idling, it would be very
> permissible to shut things down when they are not in use as the system
> does not need a lot of performance at that point.
>
> I've suggested hooking power-domain support into the regulator framework
> but Mark's pointed out some problems there. it may be worth using the
> regulator framework as a low-level system so that regulators can be
> chained off (ie, control external ldos/switches/etc).

Um. yes.. I also thought about supporting powerdomain at regulator
framework, but the tight coupling between clocks and
powerdomains/blockgating combined with the problem of ALWAYS_ON and
BOOT_ON clocks makes it inadequate; we need to monitor the states of
related clocks even if those clocks were never "clk_enable"d. Because
we need to look at the related clocks before turning on and off
powerdomain, I think powerdomain would be better reside in clock
framework or machine implementation. Otherwise, we'll end up coupling
another framework with clock framework deeply.

Or... maybe _if_ it's ok to access clock-related registers at
regulator framework directly without accessing via clock framework (I
don't like this idea.. but), we may be able to implement powerdomain
at regulator framework by allowing regulator framework to look at the
states of related clocks. In that way, we can avoid turning off
powerdomains that should not be.

>
> I'd be happier to see something where the device drivers use runtime pm
> (possibly with the regulator framework) to allow the user to stop the
> system. We could also look at device in-use doing auto runtime-pm to
> say open => resume, and close => suspend.
>

This way we are putting more burdens to drivers (at least they need to
add regulators and enable/disable them), but I agree that this would
help implement realtime_pm.




>>> Part (1) is a definite barrier to merging, the code whilst functional is
>>> both ugly and the use of #ifdefs like that are contrary to a number of
>>> developers beliefs.
>>>
>>> The second part is also a stumbling block, as it is likely to be
>>> commented
>>> upon by a number of interested parties if it did come up for review. I am
>>> certainly not happy to see this sort of invasive change to the clock
>>> system
>>> merged. I expect others would also raise objections.
>>>
>>> ...
>>>
>>> ===
>>>
>>> So need to another approach for it.
>
> It certainly needs a pile of work before we can think about it.
>
>>> Thanks.
>>>
>>> Best regards,
>>> Kgene.
>>> --
>>> Kukjin Kim<kgene.kim@samsung.com>, Senior Engineer,
>>> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>
> --
> Ben Dooks, Design & Software Engineer, Simtec Electronics
>
> http://www.simtec.co.uk/
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858