[PATCH 0/3] Power-domain clk handling

[PATCH 0/3] Power-domain clk handling

[Arun Kumar K]

This patchset enables the clk handling in power domain for
working as per the recommended power domain on / off sequence for
exynos5 SoCs. I have posted an RFC for the same [1] and didnt get any
review comments / objections. So I am dropping the RFC tag and
posting the patch along with the required clk and dt support.

[1] https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg30479.html

Arun Kumar K (2):
  clk: exynos5420: Add IDs for clocks used in PD mfc
  ARM: dts: Add clock property for mfc_pd in 5420

Prathyush K (1):
  ARM: EXYNOS: Add support for clock handling in power domain

 .../bindings/arm/exynos/power_domain.txt           |   18 +++++++
 arch/arm/boot/dts/exynos5420.dtsi                  |    3 ++
 arch/arm/mach-exynos/pm_domains.c                  |   56 +++++++++++++++++++-
 drivers/clk/samsung/clk-exynos5420.c               |    6 ++-
 include/dt-bindings/clock/exynos5420.h             |    2 +
 5 files changed, 82 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] ARM: EXYNOS: Add support for clock handling in power domain

[Arun Kumar K]

From: Prathyush K <prathyush.k@samsung.com>

While powering on/off a local powerdomain in exynos5 chipsets, the input
clocks to each device gets modified. This behaviour is based on the
SYSCLK_SYS_PWR_REG registers.
E.g. SYSCLK_MFC_SYS_PWR_REG = 0x0, the parent of input clock to MFC
				   (aclk333) gets modified to oscclk
			    = 0x1, no change in clocks.
The recommended value of SYSCLK_SYS_PWR_REG before power gating any
domain is 0x0. So we must also restore the clocks while powering on a
domain everytime.

This patch adds the framework for getting the required mux and parent clocks
through a power domain device node. With this patch, while powering off
a domain, parent is set to oscclk and while powering back on, its re-set
to the correct parent which is as per the recommended pd on/off
sequence.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           |   18 +++++++
 arch/arm/mach-exynos/pm_domains.c                  |   56 +++++++++++++++++++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 5216b41..168a191 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -9,6 +9,16 @@ Required Properties:
 - reg: physical base address of the controller and length of memory mapped
     region.
 
+Optional Properties:
+- clocks: List of clock handles. The parent clocks of the input clocks to the
+  devices in this power domain are set to oscclk before power gating and
+  restored back after powering on a domain. This is required for all domains
+  which are powered on and off and not required for unused domains.
+  The following clocks can be specified:
+  - oscclk: oscillator clock.
+  - clk(n): input clock to the devices in this power domain
+  - pclk(n): parent clock of input clock to the devices in this power domain
+
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
 
@@ -19,6 +29,14 @@ Example:
 		reg = <0x10023C00 0x10>;
 	};
 
+	mfc_pd: power-domain@10044060 {
+		compatible = "samsung,exynos4210-pd";
+		reg = <0x10044060 0x20>;
+		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
+			<&clock CLK_MOUT_USER_ACLK333>;
+		clock-names = "oscclk", "pclk0", "clk0";
+	};
+
 Example of the node using power domain:
 
 	node {
diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index fe6570e..e5fe76d 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/pm_domain.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
@@ -24,6 +25,8 @@
 
 #include "regs-pmu.h"
 
+#define MAX_CLK_PER_DOMAIN			4
+
 /*
  * Exynos specific wrapper around the generic power domain
  */
@@ -32,6 +35,9 @@ struct exynos_pm_domain {
 	char const *name;
 	bool is_off;
 	struct generic_pm_domain pd;
+	struct clk *oscclk;
+	struct clk *clk[MAX_CLK_PER_DOMAIN];
+	struct clk *pclk[MAX_CLK_PER_DOMAIN];
 };
 
 static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
@@ -44,6 +50,18 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 	pd = container_of(domain, struct exynos_pm_domain, pd);
 	base = pd->base;
 
+	/* Set oscclk before powering off a domain*/
+	if (!power_on) {
+		int i;
+		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
+			if (!pd->clk[i])
+				break;
+			if (clk_set_parent(pd->clk[i], pd->oscclk))
+				pr_info("%s: error setting oscclk as parent to clock %d\n",
+						pd->name, i);
+		}
+	}
+
 	pwr = power_on ? S5P_INT_LOCAL_PWR_EN : 0;
 	__raw_writel(pwr, base);
 
@@ -60,6 +78,19 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 		cpu_relax();
 		usleep_range(80, 100);
 	}
+
+	/* Restore clocks after powering on a domain*/
+	if (power_on) {
+		int i;
+		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
+			if (!pd->clk[i])
+				break;
+			if (clk_set_parent(pd->clk[i], pd->pclk[i]))
+				pr_info("%s: error setting parent to clock%d\n",
+						pd->name, i);
+		}
+	}
+
 	return 0;
 }
 
@@ -152,9 +183,11 @@ static __init int exynos4_pm_init_power_domain(void)
 
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
 		struct exynos_pm_domain *pd;
-		int on;
+		int on, i;
+		struct device *dev;
 
 		pdev = of_find_device_by_node(np);
+		dev = &pdev->dev;
 
 		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
 		if (!pd) {
@@ -170,6 +203,27 @@ static __init int exynos4_pm_init_power_domain(void)
 		pd->pd.power_on = exynos_pd_power_on;
 		pd->pd.of_node = np;
 
+		pd->oscclk = devm_clk_get(dev, "oscclk");
+		if (IS_ERR(pd->oscclk))
+			goto no_clk;
+
+		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
+			struct clk *tmp, *tmp_parent;
+			char clk_name[8];
+
+			snprintf(clk_name, sizeof(clk_name), "clk%d", i);
+			tmp = devm_clk_get(dev, clk_name);
+			if (IS_ERR(tmp))
+				break;
+			snprintf(clk_name, sizeof(clk_name), "pclk%d", i);
+			tmp_parent = devm_clk_get(dev, clk_name);
+			if (IS_ERR(tmp_parent))
+				break;
+			pd->clk[i] = tmp;
+			pd->pclk[i] = tmp_parent;
+		}
+
+no_clk:
 		platform_set_drvdata(pdev, pd);
 
 		on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
-- 
1.7.9.5

Re: [PATCH 1/3] ARM: EXYNOS: Add support for clock handling in power domain

[Tomasz Figa]

Hi,

On 23.05.2014 07:08, Arun Kumar K wrote:
> From: Prathyush K <prathyush.k@samsung.com>
> 
> While powering on/off a local powerdomain in exynos5 chipsets, the input
> clocks to each device gets modified. This behaviour is based on the
> SYSCLK_SYS_PWR_REG registers.
> E.g. SYSCLK_MFC_SYS_PWR_REG = 0x0, the parent of input clock to MFC
> 				   (aclk333) gets modified to oscclk
> 			    = 0x1, no change in clocks.
> The recommended value of SYSCLK_SYS_PWR_REG before power gating any
> domain is 0x0. So we must also restore the clocks while powering on a
> domain everytime.
> 
> This patch adds the framework for getting the required mux and parent clocks
> through a power domain device node. With this patch, while powering off
> a domain, parent is set to oscclk and while powering back on, its re-set
> to the correct parent which is as per the recommended pd on/off
> sequence.
> 
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  .../bindings/arm/exynos/power_domain.txt           |   18 +++++++
>  arch/arm/mach-exynos/pm_domains.c                  |   56 +++++++++++++++++++-
>  2 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> index 5216b41..168a191 100644
> --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> @@ -9,6 +9,16 @@ Required Properties:
>  - reg: physical base address of the controller and length of memory mapped
>      region.
>  
> +Optional Properties:
> +- clocks: List of clock handles. The parent clocks of the input clocks to the
> +  devices in this power domain are set to oscclk before power gating and
> +  restored back after powering on a domain. This is required for all domains
> +  which are powered on and off and not required for unused domains.

I'd keep it required to all domains that exhibit this behavior, as
device tree should expose complete information about the hardware
whenever possible.

> +  The following clocks can be specified:
> +  - oscclk: oscillator clock.
> +  - clk(n): input clock to the devices in this power domain

s/clk(n)/clkN/

> +  - pclk(n): parent clock of input clock to the devices in this power domain

s/pclk(n)/pclkN/

The meaning of N should be described and the relation between clkN and
pclkN with the same value of N.

Also shouldn't this rather be a description of clock-names property?

> +
>  Node of a device using power domains must have a samsung,power-domain property
>  defined with a phandle to respective power domain.
>  
> @@ -19,6 +29,14 @@ Example:
>  		reg = <0x10023C00 0x10>;
>  	};
>  
> +	mfc_pd: power-domain@10044060 {
> +		compatible = "samsung,exynos4210-pd";
> +		reg = <0x10044060 0x20>;
> +		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
> +			<&clock CLK_MOUT_USER_ACLK333>;
> +		clock-names = "oscclk", "pclk0", "clk0";
> +	};
> +
>  Example of the node using power domain:
>  
>  	node {
> diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
> index fe6570e..e5fe76d 100644
> --- a/arch/arm/mach-exynos/pm_domains.c
> +++ b/arch/arm/mach-exynos/pm_domains.c
> @@ -17,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/pm_domain.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
> @@ -24,6 +25,8 @@
>  
>  #include "regs-pmu.h"
>  
> +#define MAX_CLK_PER_DOMAIN			4
> +
>  /*
>   * Exynos specific wrapper around the generic power domain
>   */
> @@ -32,6 +35,9 @@ struct exynos_pm_domain {
>  	char const *name;
>  	bool is_off;
>  	struct generic_pm_domain pd;
> +	struct clk *oscclk;
> +	struct clk *clk[MAX_CLK_PER_DOMAIN];
> +	struct clk *pclk[MAX_CLK_PER_DOMAIN];
>  };
>  
>  static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
> @@ -44,6 +50,18 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
>  	pd = container_of(domain, struct exynos_pm_domain, pd);
>  	base = pd->base;
>  
> +	/* Set oscclk before powering off a domain*/
> +	if (!power_on) {
> +		int i;
> +		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
> +			if (!pd->clk[i])
> +				break;

Clock handles should be checked for validity using IS_ERR() macro (as
most of opaque handles, which should not be considered pointers, even if
they have a pointer type).

> +			if (clk_set_parent(pd->clk[i], pd->oscclk))
> +				pr_info("%s: error setting oscclk as parent to clock %d\n",
> +						pd->name, i);

pr_err()?

> +		}
> +	}
> +
>  	pwr = power_on ? S5P_INT_LOCAL_PWR_EN : 0;
>  	__raw_writel(pwr, base);
>  
> @@ -60,6 +78,19 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
>  		cpu_relax();
>  		usleep_range(80, 100);
>  	}
> +
> +	/* Restore clocks after powering on a domain*/
> +	if (power_on) {
> +		int i;
> +		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
> +			if (!pd->clk[i])
> +				break;

Ditto.

> +			if (clk_set_parent(pd->clk[i], pd->pclk[i]))
> +				pr_info("%s: error setting parent to clock%d\n",
> +						pd->name, i);

Ditto.

> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -152,9 +183,11 @@ static __init int exynos4_pm_init_power_domain(void)
>  
>  	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
>  		struct exynos_pm_domain *pd;
> -		int on;
> +		int on, i;
> +		struct device *dev;
>  
>  		pdev = of_find_device_by_node(np);
> +		dev = &pdev->dev;
>  
>  		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>  		if (!pd) {
> @@ -170,6 +203,27 @@ static __init int exynos4_pm_init_power_domain(void)
>  		pd->pd.power_on = exynos_pd_power_on;
>  		pd->pd.of_node = np;
>  
> +		pd->oscclk = devm_clk_get(dev, "oscclk");

Based on the fact that power domains are not hotpluggable (and not even
handled by a normal driver in terms of driver core), I don't see a point
in using managed resources.

> +		if (IS_ERR(pd->oscclk))
> +			goto no_clk;
> +
> +		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
> +			struct clk *tmp, *tmp_parent;
> +			char clk_name[8];
> +
> +			snprintf(clk_name, sizeof(clk_name), "clk%d", i);
> +			tmp = devm_clk_get(dev, clk_name);

Ditto.

> +			if (IS_ERR(tmp))
> +				break;
> +			snprintf(clk_name, sizeof(clk_name), "pclk%d", i);
> +			tmp_parent = devm_clk_get(dev, clk_name);

Ditto.

> +			if (IS_ERR(tmp_parent))
> +				break;
> +			pd->clk[i] = tmp;
> +			pd->pclk[i] = tmp_parent;

You should use those directly to store return values of calls to
clk_get(), to be able check clock handle validity properly.

Best regards,
Tomasz

[PATCH 2/3] clk: exynos5420: Add IDs for clocks used in PD mfc

[Arun Kumar K]

Adds IDs for MUX clocks to be used by power domain for MFC
for doing re-parenting while pd on/off.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/clk/samsung/clk-exynos5420.c   |    6 ++++--
 include/dt-bindings/clock/exynos5420.h |    2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 9d7d7ee..c899dbe 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -631,7 +631,8 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
 			SRC_TOP4, 16, 1),
 	MUX(0, "mout_user_aclk266", mout_user_aclk266_p, SRC_TOP4, 20, 1),
 	MUX(0, "mout_user_aclk166", mout_user_aclk166_p, SRC_TOP4, 24, 1),
-	MUX(0, "mout_user_aclk333", mout_user_aclk333_p, SRC_TOP4, 28, 1),
+	MUX(CLK_MOUT_USER_ACLK333, "mout_user_aclk333", mout_user_aclk333_p,
+							SRC_TOP4, 28, 1),
 
 	MUX(0, "mout_user_aclk400_disp1", mout_user_aclk400_disp1_p,
 			SRC_TOP5, 0, 1),
@@ -684,7 +685,8 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
 			SRC_TOP11, 12, 1),
 	MUX(0, "mout_sw_aclk266", mout_sw_aclk266_p, SRC_TOP11, 20, 1),
 	MUX(0, "mout_sw_aclk166", mout_sw_aclk166_p, SRC_TOP11, 24, 1),
-	MUX(0, "mout_sw_aclk333", mout_sw_aclk333_p, SRC_TOP11, 28, 1),
+	MUX(CLK_MOUT_SW_ACLK333, "mout_sw_aclk333", mout_sw_aclk333_p,
+						SRC_TOP11, 28, 1),
 
 	MUX(0, "mout_sw_aclk400_disp1", mout_sw_aclk400_disp1_p,
 			SRC_TOP12, 4, 1),
diff --git a/include/dt-bindings/clock/exynos5420.h b/include/dt-bindings/clock/exynos5420.h
index 97dcb89..3fc08ff 100644
--- a/include/dt-bindings/clock/exynos5420.h
+++ b/include/dt-bindings/clock/exynos5420.h
@@ -203,6 +203,8 @@
 #define CLK_MOUT_G3D		641
 #define CLK_MOUT_VPLL		642
 #define CLK_MOUT_MAUDIO0	643
+#define CLK_MOUT_USER_ACLK333	644
+#define CLK_MOUT_SW_ACLK333	645
 
 /* divider clocks */
 #define CLK_DOUT_PIXEL		768
-- 
1.7.9.5

Re: [PATCH 2/3] clk: exynos5420: Add IDs for clocks used in PD mfc

[Tomasz Figa]

Hi Arun,

On 23.05.2014 07:08, Arun Kumar K wrote:
> Adds IDs for MUX clocks to be used by power domain for MFC
> for doing re-parenting while pd on/off.
> 
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5420.c   |    6 ++++--
>  include/dt-bindings/clock/exynos5420.h |    2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index 9d7d7ee..c899dbe 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -631,7 +631,8 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
>  			SRC_TOP4, 16, 1),
>  	MUX(0, "mout_user_aclk266", mout_user_aclk266_p, SRC_TOP4, 20, 1),
>  	MUX(0, "mout_user_aclk166", mout_user_aclk166_p, SRC_TOP4, 24, 1),
> -	MUX(0, "mout_user_aclk333", mout_user_aclk333_p, SRC_TOP4, 28, 1),
> +	MUX(CLK_MOUT_USER_ACLK333, "mout_user_aclk333", mout_user_aclk333_p,
> +							SRC_TOP4, 28, 1),

Please keep the indentation consistent. As you can see below, just two
extra tabs are used on wrapped lines.

>  
>  	MUX(0, "mout_user_aclk400_disp1", mout_user_aclk400_disp1_p,
>  			SRC_TOP5, 0, 1),
> @@ -684,7 +685,8 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
>  			SRC_TOP11, 12, 1),
>  	MUX(0, "mout_sw_aclk266", mout_sw_aclk266_p, SRC_TOP11, 20, 1),
>  	MUX(0, "mout_sw_aclk166", mout_sw_aclk166_p, SRC_TOP11, 24, 1),
> -	MUX(0, "mout_sw_aclk333", mout_sw_aclk333_p, SRC_TOP11, 28, 1),
> +	MUX(CLK_MOUT_SW_ACLK333, "mout_sw_aclk333", mout_sw_aclk333_p,
> +						SRC_TOP11, 28, 1),

Ditto.

Best regards,
Tomasz

[PATCH 3/3] ARM: dts: Add clock property for mfc_pd in 5420

[Arun Kumar K]

Adding the optional clock property for the mfc_pd for
handling the re-parenting while pd on/off.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 5a85896..890bdac 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -260,6 +260,9 @@
 	mfc_pd: power-domain@10044060 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10044060 0x20>;
+		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
+			<&clock CLK_MOUT_USER_ACLK333>,;
+		clock-names = "oscclk", "pclk0", "clk0";
 	};
 
 	disp_pd: power-domain@100440C0 {
-- 
1.7.9.5

Re: [PATCH 3/3] ARM: dts: Add clock property for mfc_pd in 5420

[Tomasz Figa]

On 23.05.2014 07:08, Arun Kumar K wrote:
> Adding the optional clock property for the mfc_pd for
> handling the re-parenting while pd on/off.
> 
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5420.dtsi |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 5a85896..890bdac 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -260,6 +260,9 @@
>  	mfc_pd: power-domain@10044060 {
>  		compatible = "samsung,exynos4210-pd";
>  		reg = <0x10044060 0x20>;
> +		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
> +			<&clock CLK_MOUT_USER_ACLK333>,;
> +		clock-names = "oscclk", "pclk0", "clk0";
>  	};
>  
>  	disp_pd: power-domain@100440C0 {
> 

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz