[PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[Marek Szyprowski]

Hi!

For everyone working on power management and runtime power management it
is important to have a meaningful information about the state of the
power domains. Current Exynos power domain driver created names of the
domains based on the device tree node name. Those name
we incorrectly a bit more descriptive (like "mfc-power-domain@10023C40"
in exynos4.dtsi) than they should be (it should be fixed to
"power-domain@10023C40"). This patch series adds reading human readable
names from the 'label' property. While touching this, I've also fixes
a few obvious issues related to power domain driver code.

Patches are based on the Linux next-20170130 branch.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:
v3:
- moved real domains names from hardcoded string in the driver to the label
  property in the device tree (suggested by Krzysztof Kozlowski)

v2:
- sorted all domains data by domain base address in the arrays in driver

v1:
- initial version


Patch summary:

Marek Szyprowski (5):
  soc: samsung: pm_domains: Use full names in subdomains registration
    log
  soc: samsung: pm_domains: Remove unused name field
  soc: samsung: pm_domains: Remove message about failed memory
    allocation
  soc: samsung: pm_domains: Read domain name from the new label property
  arm: dts: exynos: Add labels to all existing power domains

 .../devicetree/bindings/power/pd-samsung.txt       |  4 ++++
 arch/arm/boot/dts/exynos4.dtsi                     |  7 +++++++
 arch/arm/boot/dts/exynos4210.dtsi                  |  1 +
 arch/arm/boot/dts/exynos4412.dtsi                  |  1 +
 arch/arm/boot/dts/exynos5250.dtsi                  |  3 +++
 arch/arm/boot/dts/exynos5420.dtsi                  |  5 +++++
 drivers/soc/samsung/pm_domains.c                   | 24 +++++++++++++---------
 7 files changed, 35 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH v3 1/5] soc: samsung: pm_domains: Use full names in subdomains registration log

[Marek Szyprowski]

Device tree none name for each power domain should be "power-domain", so
use a bit more descriptive full node name in messages about subdomain
registration. This way the following meaningless message:

power-domain has as child subdomain: power-domain.

is changed to a bit more meaningful one:

/soc/power-domain@105c40a0 has as child subdomain: /soc/power-domain@105c4020.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/soc/samsung/pm_domains.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 15bad1543409..c33196087e9f 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -234,10 +234,10 @@ static __init int exynos4_pm_init_power_domain(void)
 
 		if (of_genpd_add_subdomain(&parent, &child))
 			pr_warn("%s failed to add subdomain: %s\n",
-				parent.np->name, child.np->name);
+				parent.np->full_name, child.np->full_name);
 		else
 			pr_info("%s has as child subdomain: %s.\n",
-				parent.np->name, child.np->name);
+				parent.np->full_name, child.np->full_name);
 	}
 
 	return 0;
-- 
1.9.1

Re: [PATCH v3 1/5] soc: samsung: pm_domains: Use full names in subdomains registration log

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:18:56PM +0100, Marek Szyprowski wrote:
> Device tree none name for each power domain should be "power-domain", so
> use a bit more descriptive full node name in messages about subdomain
> registration. This way the following meaningless message:
> 
> power-domain has as child subdomain: power-domain.
> 
> is changed to a bit more meaningful one:
> 
> /soc/power-domain@105c40a0 has as child subdomain: /soc/power-domain@105c4020.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/soc/samsung/pm_domains.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH v3 2/5] soc: samsung: pm_domains: Remove unused name field

[Marek Szyprowski]

Name is now in generic pm domain structure, so there is no need to
duplicate it in private data.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/soc/samsung/pm_domains.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index c33196087e9f..00e32772655a 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -35,7 +35,6 @@ struct exynos_pm_domain_config {
  */
 struct exynos_pm_domain {
 	void __iomem *base;
-	char const *name;
 	bool is_off;
 	struct generic_pm_domain pd;
 	struct clk *oscclk;
@@ -70,7 +69,7 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 			pd->pclk[i] = clk_get_parent(pd->clk[i]);
 			if (clk_set_parent(pd->clk[i], pd->oscclk))
 				pr_err("%s: error setting oscclk as parent to clock %d\n",
-						pd->name, i);
+						domain->name, i);
 		}
 	}
 
@@ -101,7 +100,7 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 				continue; /* Skip on first power up */
 			if (clk_set_parent(pd->clk[i], pd->pclk[i]))
 				pr_err("%s: error setting parent to clock%d\n",
-						pd->name, i);
+						domain->name, i);
 		}
 	}
 
@@ -170,7 +169,6 @@ static __init int exynos4_pm_init_power_domain(void)
 			return -ENOMEM;
 		}
 
-		pd->name = pd->pd.name;
 		pd->base = of_iomap(np, 0);
 		if (!pd->base) {
 			pr_warn("%s: failed to map memory\n", __func__);
-- 
1.9.1

Re: [PATCH v3 2/5] soc: samsung: pm_domains: Remove unused name field

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:18:57PM +0100, Marek Szyprowski wrote:
> Name is now in generic pm domain structure, so there is no need to
> duplicate it in private data.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/soc/samsung/pm_domains.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH v3 3/5] soc: samsung: pm_domains: Remove message about failed memory allocation

[Marek Szyprowski]

Memory subsystem already prints message about failed memory
allocation, there is no need to do it in the drivers.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/soc/samsung/pm_domains.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 00e32772655a..5a0a46bcbe18 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -156,8 +156,6 @@ static __init int exynos4_pm_init_power_domain(void)
 
 		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
 		if (!pd) {
-			pr_err("%s: failed to allocate memory for domain\n",
-					__func__);
 			of_node_put(np);
 			return -ENOMEM;
 		}
-- 
1.9.1

Re: [PATCH v3 3/5] soc: samsung: pm_domains: Remove message about failed memory allocation

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:18:58PM +0100, Marek Szyprowski wrote:
> Memory subsystem already prints message about failed memory
> allocation, there is no need to do it in the drivers.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/soc/samsung/pm_domains.c | 2 --
>  1 file changed, 2 deletions(-)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH v3 4/5] soc: samsung: pm_domains: Read domain name from the new label property

[Marek Szyprowski]

Device tree nodes for each power domain should use generic "power-domain"
name, so using it as a domain name doesn't give much benefits. This patch
adds support for human readable names defined in 'label' property. Such
names are visible to userspace and makes debugging much easier. When no
'label' property is found, driver keeps using the name constructed from
full node name.

Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 Documentation/devicetree/bindings/power/pd-samsung.txt |  4 ++++
 drivers/soc/samsung/pm_domains.c                       | 12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/pd-samsung.txt b/Documentation/devicetree/bindings/power/pd-samsung.txt
index fb08c8d62733..549f7dee9b9d 100644
--- a/Documentation/devicetree/bindings/power/pd-samsung.txt
+++ b/Documentation/devicetree/bindings/power/pd-samsung.txt
@@ -13,6 +13,8 @@ Required Properties:
     must be 0.
 
 Optional Properties:
+- label: Human readable string with domain name. Will be visible in userspace
+	to let user to distinguish between multiple domains in SoC.
 - 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
@@ -39,6 +41,7 @@ Example:
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C00 0x10>;
 		#power-domain-cells = <0>;
+		label = "LCD0";
 	};
 
 	mfc_pd: power-domain@10044060 {
@@ -47,6 +50,7 @@ Example:
 		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>;
 		clock-names = "oscclk", "clk0";
 		#power-domain-cells = <0>;
+		label = "MFC";
 	};
 
 See Documentation/devicetree/bindings/power/power_domain.txt for description
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 5a0a46bcbe18..a6a5d807cc2b 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -142,6 +142,15 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	{ },
 };
 
+static __init const char *exynos_get_domain_name(struct device_node *node)
+{
+	const char *name;
+
+	if (of_property_read_string(node, "label", &name) < 0)
+		name = strrchr(node->full_name, '/') + 1;
+	return kstrdup_const(name, GFP_KERNEL);
+}
+
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct device_node *np;
@@ -159,8 +168,7 @@ static __init int exynos4_pm_init_power_domain(void)
 			of_node_put(np);
 			return -ENOMEM;
 		}
-		pd->pd.name = kstrdup_const(strrchr(np->full_name, '/') + 1,
-					    GFP_KERNEL);
+		pd->pd.name = exynos_get_domain_name(np);
 		if (!pd->pd.name) {
 			kfree(pd);
 			of_node_put(np);
-- 
1.9.1

Re: [PATCH v3 4/5] soc: samsung: pm_domains: Read domain name from the new label property

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:18:59PM +0100, Marek Szyprowski wrote:
> Device tree nodes for each power domain should use generic "power-domain"
> name, so using it as a domain name doesn't give much benefits. This patch
> adds support for human readable names defined in 'label' property. Such
> names are visible to userspace and makes debugging much easier. When no
> 'label' property is found, driver keeps using the name constructed from
> full node name.
> 
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  Documentation/devicetree/bindings/power/pd-samsung.txt |  4 ++++
>  drivers/soc/samsung/pm_domains.c                       | 12 ++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 

Thanks, applied.

Best regards,
Krzysztof

Re: [PATCH v3 4/5] soc: samsung: pm_domains: Read domain name from the new label property

[Rob Herring]

On Mon, Jan 30, 2017 at 01:18:59PM +0100, Marek Szyprowski wrote:
> Device tree nodes for each power domain should use generic "power-domain"
> name, so using it as a domain name doesn't give much benefits. This patch
> adds support for human readable names defined in 'label' property. Such
> names are visible to userspace and makes debugging much easier. When no
> 'label' property is found, driver keeps using the name constructed from
> full node name.

I'm not sure this is really a good use of label. label is intended for 
end user visible things like ports/connectors on a board. It's fine 
here, but I wouldn't want to see it used everywhere.

> 
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  Documentation/devicetree/bindings/power/pd-samsung.txt |  4 ++++
>  drivers/soc/samsung/pm_domains.c                       | 12 ++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)

Re: [PATCH v3 4/5] soc: samsung: pm_domains: Read domain name from the new label property

[Krzysztof Kozlowski]

On Wed, Feb 01, 2017 at 10:25:54AM -0600, Rob Herring wrote:
> On Mon, Jan 30, 2017 at 01:18:59PM +0100, Marek Szyprowski wrote:
> > Device tree nodes for each power domain should use generic "power-domain"
> > name, so using it as a domain name doesn't give much benefits. This patch
> > adds support for human readable names defined in 'label' property. Such
> > names are visible to userspace and makes debugging much easier. When no
> > 'label' property is found, driver keeps using the name constructed from
> > full node name.
> 
> I'm not sure this is really a good use of label. label is intended for 
> end user visible things like ports/connectors on a board. It's fine 
> here, but I wouldn't want to see it used everywhere.
>

Thanks for comments. We want to use the label here for user visible
output - through dmesg or debugfs. Otherwise, we would have to stick
(always) to more descriptive node names, like "lcd-power-domain@xxxxx".

Best regards,
Krzysztof

[PATCH v3 5/5] arm: dts: exynos: Add labels to all existing power domains

[Marek Szyprowski]

Provide human readable names for all power domains defined in Exynos SoCs.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi    | 7 +++++++
 arch/arm/boot/dts/exynos4210.dtsi | 1 +
 arch/arm/boot/dts/exynos4412.dtsi | 1 +
 arch/arm/boot/dts/exynos5250.dtsi | 3 +++
 arch/arm/boot/dts/exynos5420.dtsi | 5 +++++
 5 files changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 8fb0e00dd1be..18def1c774d5 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -102,18 +102,21 @@
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C40 0x20>;
 		#power-domain-cells = <0>;
+		label = "MFC";
 	};
 
 	pd_g3d: g3d-power-domain@10023C60 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C60 0x20>;
 		#power-domain-cells = <0>;
+		label = "G3D";
 	};
 
 	pd_lcd0: lcd0-power-domain@10023C80 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C80 0x20>;
 		#power-domain-cells = <0>;
+		label = "LCD0";
 	};
 
 	pd_tv: tv-power-domain@10023C20 {
@@ -121,24 +124,28 @@
 		reg = <0x10023C20 0x20>;
 		#power-domain-cells = <0>;
 		power-domains = <&pd_lcd0>;
+		label = "TV";
 	};
 
 	pd_cam: cam-power-domain@10023C00 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C00 0x20>;
 		#power-domain-cells = <0>;
+		label = "CAM";
 	};
 
 	pd_gps: gps-power-domain@10023CE0 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023CE0 0x20>;
 		#power-domain-cells = <0>;
+		label = "GPS";
 	};
 
 	pd_gps_alive: gps-alive-power-domain@10023D00 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023D00 0x20>;
 		#power-domain-cells = <0>;
+		label = "GPS alive";
 	};
 
 	gic: interrupt-controller@10490000 {
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index 7f3a18c8f60f..f9408188f97f 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -86,6 +86,7 @@
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023CA0 0x20>;
 		#power-domain-cells = <0>;
+		label = "LCD1";
 	};
 
 	l2c: l2-cache-controller@10502000 {
diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index 4f7b5a1a848c..235bbb69ad7c 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -172,6 +172,7 @@
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023CA0 0x20>;
 		#power-domain-cells = <0>;
+		label = "ISP";
 	};
 
 	l2c: l2-cache-controller@10502000 {
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 0e04460a815a..79c9c885613a 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -115,18 +115,21 @@
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044000 0x20>;
 			#power-domain-cells = <0>;
+			label = "GSC";
 		};
 
 		pd_mfc: mfc-power-domain@10044040 {
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044040 0x20>;
 			#power-domain-cells = <0>;
+			label = "MFC";
 		};
 
 		pd_disp1: disp1-power-domain@100440A0 {
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x100440A0 0x20>;
 			#power-domain-cells = <0>;
+			label = "DISP1";
 			clocks = <&clock CLK_FIN_PLL>,
 				 <&clock CLK_MOUT_ACLK200_DISP1_SUB>,
 				 <&clock CLK_MOUT_ACLK300_DISP1_SUB>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 0154c2e373f8..7dc9dc82afd8 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -277,6 +277,7 @@
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044000 0x20>;
 			#power-domain-cells = <0>;
+			label = "GSC";
 			clocks = <&clock CLK_FIN_PLL>,
 				 <&clock CLK_MOUT_USER_ACLK300_GSCL>,
 				 <&clock CLK_GSCL0>, <&clock CLK_GSCL1>;
@@ -287,6 +288,7 @@
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044020 0x20>;
 			#power-domain-cells = <0>;
+			label = "ISP";
 		};
 
 		mfc_pd: power-domain@10044060 {
@@ -297,18 +299,21 @@
 				 <&clock CLK_ACLK333>;
 			clock-names = "oscclk", "clk0","asb0";
 			#power-domain-cells = <0>;
+			label = "MFC";
 		};
 
 		msc_pd: power-domain@10044120 {
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044120 0x20>;
 			#power-domain-cells = <0>;
+			label = "MSC";
 		};
 
 		disp_pd: power-domain@100440C0 {
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x100440C0 0x20>;
 			#power-domain-cells = <0>;
+			label = "DISP";
 			clocks = <&clock CLK_FIN_PLL>,
 				 <&clock CLK_MOUT_USER_ACLK200_DISP1>,
 				 <&clock CLK_MOUT_USER_ACLK300_DISP1>,
-- 
1.9.1

Re: [PATCH v3 5/5] arm: dts: exynos: Add labels to all existing power domains

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:19:00PM +0100, Marek Szyprowski wrote:
> Provide human readable names for all power domains defined in Exynos SoCs.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4.dtsi    | 7 +++++++
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  arch/arm/boot/dts/exynos4412.dtsi | 1 +
>  arch/arm/boot/dts/exynos5250.dtsi | 3 +++
>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++++
>  5 files changed, 17 insertions(+)
> 

Thanks, applied.

Best regards,
Krzysztof

Re: [PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[Krzysztof Kozlowski]

On Mon, Jan 30, 2017 at 01:18:55PM +0100, Marek Szyprowski wrote:
> Hi!
> 
> For everyone working on power management and runtime power management it
> is important to have a meaningful information about the state of the
> power domains. Current Exynos power domain driver created names of the
> domains based on the device tree node name. Those name
> we incorrectly a bit more descriptive (like "mfc-power-domain@10023C40"
> in exynos4.dtsi) than they should be (it should be fixed to
> "power-domain@10023C40"). This patch series adds reading human readable
> names from the 'label' property. While touching this, I've also fixes
> a few obvious issues related to power domain driver code.
> 
> Patches are based on the Linux next-20170130 branch.
> 
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
>

I think I am missing something... but it looks it does not work. I
applied it on top of my for-next tree and:
cat /sys/kernel/debug/pm_genpd/pm_genpd_summary 
domain                          status          slaves
    /device                                             runtime status
    ----------------------------------------------------------------------
    isp-power-domain@10023CA0       off-0           
    gps-alive-power-domain@10023D00  off-0           
    gps-power-domain@10023CE0       off-0           
    cam-power-domain@10023C00       off-0           

Also, with enabled debug on pm-domains and unbinding a device:
[  180.016800] s5p-mfc 13400000.codec: genpd_runtime_resume()
[  180.017289] s5p-mfc 13400000.codec: genpd_runtime_suspend()
[  180.017519] s5p-mfc 13400000.codec: Removing 13400000.codec
[  180.024283] s5p-mfc 13400000.codec: removing from PM domain mfc-power-domain@10023C40
[  180.024323] s5p-mfc 13400000.codec: genpd_remove_device()

I think in both cases the genpd->name is used... which should be the new
name (label).

What is wrong?

Best regards,
Krzysztof

Re: [PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[Marek Szyprowski]

On 2017-01-30 19:36, Krzysztof Kozlowski wrote:
> On Mon, Jan 30, 2017 at 01:18:55PM +0100, Marek Szyprowski wrote:
>> Hi!
>>
>> For everyone working on power management and runtime power management it
>> is important to have a meaningful information about the state of the
>> power domains. Current Exynos power domain driver created names of the
>> domains based on the device tree node name. Those name
>> we incorrectly a bit more descriptive (like "mfc-power-domain@10023C40"
>> in exynos4.dtsi) than they should be (it should be fixed to
>> "power-domain@10023C40"). This patch series adds reading human readable
>> names from the 'label' property. While touching this, I've also fixes
>> a few obvious issues related to power domain driver code.
>>
>> Patches are based on the Linux next-20170130 branch.
>>
>> Best regards
>> Marek Szyprowski
>> Samsung R&D Institute Poland
>>
> I think I am missing something... but it looks it does not work. I
> applied it on top of my for-next tree and:
> cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain                          status          slaves
>      /device                                             runtime status
>      ----------------------------------------------------------------------
>      isp-power-domain@10023CA0       off-0
>      gps-alive-power-domain@10023D00  off-0
>      gps-power-domain@10023CE0       off-0
>      cam-power-domain@10023C00       off-0
>
> Also, with enabled debug on pm-domains and unbinding a device:
> [  180.016800] s5p-mfc 13400000.codec: genpd_runtime_resume()
> [  180.017289] s5p-mfc 13400000.codec: genpd_runtime_suspend()
> [  180.017519] s5p-mfc 13400000.codec: Removing 13400000.codec
> [  180.024283] s5p-mfc 13400000.codec: removing from PM domain mfc-power-domain@10023C40
> [  180.024323] s5p-mfc 13400000.codec: genpd_remove_device()
>
> I think in both cases the genpd->name is used... which should be the new
> name (label).
>
> What is wrong?

I have no idea. Are you sure you have applied the dts patch? Here it works
fine (Odroid U3 with next-20170130 + this patchset):

domain                          status          slaves
     /device                                             runtime status
----------------------------------------------------------------------
ISP                             off-0
GPS alive                       off-0
GPS                             off-0
CAM                             off-0
TV                              on
     /devices/platform/12c10000.mixer                    active
     /devices/platform/12d00000.hdmi                     active
LCD0                            on              TV
G3D                             off-0
MFC                             off-0

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[Krzysztof Kozlowski]

On Tue, Jan 31, 2017 at 08:04:53AM +0100, Marek Szyprowski wrote:
> 
> 
> On 2017-01-30 19:36, Krzysztof Kozlowski wrote:
> > On Mon, Jan 30, 2017 at 01:18:55PM +0100, Marek Szyprowski wrote:
> > > Hi!
> > > 
> > > For everyone working on power management and runtime power management it
> > > is important to have a meaningful information about the state of the
> > > power domains. Current Exynos power domain driver created names of the
> > > domains based on the device tree node name. Those name
> > > we incorrectly a bit more descriptive (like "mfc-power-domain@10023C40"
> > > in exynos4.dtsi) than they should be (it should be fixed to
> > > "power-domain@10023C40"). This patch series adds reading human readable
> > > names from the 'label' property. While touching this, I've also fixes
> > > a few obvious issues related to power domain driver code.
> > > 
> > > Patches are based on the Linux next-20170130 branch.
> > > 
> > > Best regards
> > > Marek Szyprowski
> > > Samsung R&D Institute Poland
> > > 
> > I think I am missing something... but it looks it does not work. I
> > applied it on top of my for-next tree and:
> > cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> > domain                          status          slaves
> >      /device                                             runtime status
> >      ----------------------------------------------------------------------
> >      isp-power-domain@10023CA0       off-0
> >      gps-alive-power-domain@10023D00  off-0
> >      gps-power-domain@10023CE0       off-0
> >      cam-power-domain@10023C00       off-0
> > 
> > Also, with enabled debug on pm-domains and unbinding a device:
> > [  180.016800] s5p-mfc 13400000.codec: genpd_runtime_resume()
> > [  180.017289] s5p-mfc 13400000.codec: genpd_runtime_suspend()
> > [  180.017519] s5p-mfc 13400000.codec: Removing 13400000.codec
> > [  180.024283] s5p-mfc 13400000.codec: removing from PM domain mfc-power-domain@10023C40
> > [  180.024323] s5p-mfc 13400000.codec: genpd_remove_device()
> > 
> > I think in both cases the genpd->name is used... which should be the new
> > name (label).
> > 
> > What is wrong?
> 
> I have no idea. Are you sure you have applied the dts patch? Here it works
> fine (Odroid U3 with next-20170130 + this patchset):

Yep:
$ git log --oneline
ee85c687392b arm: dts: exynos: Add labels to all existing power domains
6c63bc30112d soc: samsung: pm_domains: Read domain name from the new label property
f4c8f7f062ca soc: samsung: pm_domains: Remove message about failed memory allocation
90d5c7c36ad3 soc: samsung: pm_domains: Remove unused name field
b8b54d5c4f5a soc: samsung: pm_domains: Use full names in subdomains registration log
95648b747071 Merge branch 'next/soc' into for-next

On the board:
$ uname -a
Linux odroidu3 4.10.0-rc2-00070-gee85c687392b-dirty

The hash matches. Let my try it on the linux-next...

BR,
Krzysztof

> 
> domain                          status          slaves
>     /device                                             runtime status
> ----------------------------------------------------------------------
> ISP                             off-0
> GPS alive                       off-0
> GPS                             off-0
> CAM                             off-0
> TV                              on
>     /devices/platform/12c10000.mixer                    active
>     /devices/platform/12d00000.hdmi                     active
> LCD0                            on              TV
> G3D                             off-0
> MFC                             off-0
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 

Re: [PATCH v3 0/5] Provide real domain names for Exynos power domain driver

[Krzysztof Kozlowski]

On Tue, Jan 31, 2017 at 09:19:55PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jan 31, 2017 at 08:04:53AM +0100, Marek Szyprowski wrote:
> > I have no idea. Are you sure you have applied the dts patch? Here it works
> > fine (Odroid U3 with next-20170130 + this patchset):
> 
> Yep:
> $ git log --oneline
> ee85c687392b arm: dts: exynos: Add labels to all existing power domains
> 6c63bc30112d soc: samsung: pm_domains: Read domain name from the new label property
> f4c8f7f062ca soc: samsung: pm_domains: Remove message about failed memory allocation
> 90d5c7c36ad3 soc: samsung: pm_domains: Remove unused name field
> b8b54d5c4f5a soc: samsung: pm_domains: Use full names in subdomains registration log
> 95648b747071 Merge branch 'next/soc' into for-next
> 
> On the board:
> $ uname -a
> Linux odroidu3 4.10.0-rc2-00070-gee85c687392b-dirty
> 
> The hash matches. Let my try it on the linux-next...
> 

I'm dumbass... The kernel was updated but I put DTB in wrong directory so
old DTB was used.

Now it works fine.

Best regards,
Krzysztof