All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Provide real domain names for Exynos power domain driver
       [not found] <CGME20170127104801eucas1p2ca06f8ba0b44e41c0f7dbf0c571bb9a3@eucas1p2.samsung.com>
@ 2017-01-27 10:47 ` Marek Szyprowski
       [not found]   ` <CGME20170127104801eucas1p19a61c607387308a59bdf53fac3efba44@eucas1p1.samsung.com>
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 10:47 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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 human readable names
for all supported Exynos SoCs by the Exynos power domain driver. While
touching this, I've also fixes a few obvious issues related to power
domain driver code.

Patches are based on the following git branch:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux for-v4.11/drivers-soc-exynos-pmu-the-joy-never-ends

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (4):
  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: Provide real name for all supported domains

 drivers/soc/samsung/pm_domains.c | 137 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 127 insertions(+), 10 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] soc: samsung: pm_domains: Use full names in subdomains registration log
       [not found]   ` <CGME20170127104801eucas1p19a61c607387308a59bdf53fac3efba44@eucas1p1.samsung.com>
@ 2017-01-27 10:47     ` Marek Szyprowski
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 10:47 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] soc: samsung: pm_domains: Remove unused name field
       [not found]   ` <CGME20170127104801eucas1p2287962bae474ca786644de4ad6a11f57@eucas1p2.samsung.com>
@ 2017-01-27 10:47     ` Marek Szyprowski
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 10:47 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] soc: samsung: pm_domains: Remove message about failed memory allocation
       [not found]   ` <CGME20170127104802eucas1p27facdbb5c2914da740f4d2ea6b87680b@eucas1p2.samsung.com>
@ 2017-01-27 10:47     ` Marek Szyprowski
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 10:47 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] soc: samsung: pm_domains: Provide real name for all supported domains
       [not found]   ` <CGME20170127104802eucas1p121f5f44f8a047ad9b5118a37f66ea032@eucas1p1.samsung.com>
@ 2017-01-27 10:47     ` Marek Szyprowski
       [not found]       ` <CGME20170127144908eucas1p14c50396f670a01c31a69ec95b636a0e1@eucas1p1.samsung.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 10:47 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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 human readable names for all supported domains, what makes debugging
much easier.

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

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 5a0a46bcbe18..67d385eaab63 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -30,6 +30,17 @@ struct exynos_pm_domain_config {
 	u32 local_pwr_cfg;
 };
 
+struct exynos_pm_domain_data {
+	const char *name;
+	u32 base;
+};
+
+struct exynos_pm_domain_soc_data {
+	const char *compatible;
+	unsigned int nr_domains;
+	const struct exynos_pm_domain_data *domains;
+};
+
 /*
  * Exynos specific wrapper around the generic power domain
  */
@@ -123,6 +134,91 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	return exynos_pd_power(domain, false);
 }
 
+static const struct exynos_pm_domain_data exynos4210_domains[] __initconst = {
+	{ "LCD1",	0x10023CA0 },
+};
+
+static const struct exynos_pm_domain_data exynos4412_domains[] __initconst = {
+	{ "CAM",	0x10023C00 },
+	{ "TV",		0x10023C20 },
+	{ "MFC",	0x10023C40 },
+	{ "G3D",	0x10023C60 },
+	{ "LCD0",	0x10023C80 },
+	{ "ISP",	0x10023CA0 },
+	{ "GPS",	0x10023CE0 },
+	{ "GPS alive",	0x10023D00 },
+};
+
+static const struct exynos_pm_domain_data exynos5250_domains[] __initconst = {
+	{ "GSCL",	0x10044000 },
+	{ "ISP",	0x10044020 },
+	{ "MFC",	0x10044040 },
+	{ "G3D",	0x10044060 },
+	{ "DISP1",	0x100440A0 },
+	{ "MAU",	0x100440C0 },
+};
+
+static const struct exynos_pm_domain_data exynos542x_domains[] __initconst = {
+	{ "SCALER",	0x10044000 },
+	{ "ISP",	0x10044020 },
+	{ "MFC",	0x10044060 },
+	{ "G3D",	0x10044080 },
+	{ "DISP1",	0x100440C0 },
+	{ "MAU",	0x100440E0 },
+	{ "G2D",	0x10044100 },
+	{ "MSCL",	0x10044120 },
+	{ "FSYS",	0x10044140 },
+	{ "PERIC",	0x100441A0 },
+	{ "CAM",	0x10045100 },
+};
+
+static const struct exynos_pm_domain_data exynos5433_domains[] __initconst = {
+	{ "GSCL",	0x105c4000 },
+	{ "MSCL",	0x105c4040 },
+	{ "DISP",	0x105c4080 },
+	{ "MFC",	0x105c4180 },
+	{ "CAM0",	0x105c4020 },
+	{ "CAM1",	0x105c40a0 },
+	{ "ISP",	0x105c4140 },
+	{ "G2D",	0x105c4120 },
+	{ "G3D",	0x105c4060 },
+	{ "AUD",	0x105c40c0 },
+	{ "FSYS",	0x105c40e0 },
+	{ "HEVC",	0x105c41c0 },
+};
+
+static const struct exynos_pm_domain_soc_data soc_domains_data[] __initconst = {
+	{ /* Exynos3250 uses a subset of 4412 domains */
+		.compatible = "samsung,exynos3250",
+		.nr_domains = ARRAY_SIZE(exynos4412_domains),
+		.domains = exynos4412_domains,
+	}, { /* first check samsung,exynos4210 to detect LCD1 domain */
+		.compatible = "samsung,exynos4210",
+		.nr_domains = ARRAY_SIZE(exynos4210_domains),
+		.domains = exynos4210_domains,
+	}, { /* remaining domains for Exynos4210 and 4412 */
+		.compatible = "samsung,exynos4",
+		.nr_domains = ARRAY_SIZE(exynos4412_domains),
+		.domains = exynos4412_domains
+	}, {
+		.compatible = "samsung,exynos5250",
+		.nr_domains = ARRAY_SIZE(exynos5250_domains),
+		.domains = exynos5250_domains,
+	}, {
+		.compatible = "samsung,exynos5420",
+		.nr_domains = ARRAY_SIZE(exynos542x_domains),
+		.domains = exynos542x_domains,
+	}, {
+		.compatible = "samsung,exynos5800",
+		.nr_domains = ARRAY_SIZE(exynos542x_domains),
+		.domains = exynos542x_domains,
+	}, {
+		.compatible = "samsung,exynos5433",
+		.nr_domains = ARRAY_SIZE(exynos5433_domains),
+		.domains = exynos5433_domains,
+	},
+};
+
 static const struct exynos_pm_domain_config exynos4210_cfg __initconst = {
 	.local_pwr_cfg		= 0x7,
 };
@@ -142,6 +238,32 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	{ },
 };
 
+static __init const char *exynos_get_domain_name(struct device_node *np)
+{
+	const struct exynos_pm_domain_soc_data *soc = soc_domains_data;
+	const __be32 *reg;
+	u64 addr;
+	int i, j;
+
+
+	reg = of_get_property(np, "reg", NULL);
+	if (!reg || (addr = of_translate_address(np, reg)) == OF_BAD_ADDR)
+		goto not_found;
+
+	for (i = 0; i < ARRAY_SIZE(soc_domains_data); i++, soc++) {
+		if (!of_machine_is_compatible(soc->compatible))
+			continue;
+
+		for (j = 0; j < soc->nr_domains; j++) {
+			if (soc->domains[j].base == addr)
+				return kstrdup_const(soc->domains[j].name,
+						     GFP_KERNEL);
+		}
+	}
+not_found:
+	return kstrdup_const(strrchr(np->full_name, '/') + 1, GFP_KERNEL);
+}
+
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct device_node *np;
@@ -159,8 +281,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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] soc: samsung: pm_domains: Provide real name for all supported domains
       [not found]       ` <CGME20170127144908eucas1p14c50396f670a01c31a69ec95b636a0e1@eucas1p1.samsung.com>
@ 2017-01-27 14:48         ` Marek Szyprowski
  2017-01-28 15:16           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-27 14:48 UTC (permalink / raw)
  To: linux-samsung-soc, linux-pm
  Cc: Marek Szyprowski, Sylwester Nawrocki, Krzysztof Kozlowski,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

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 human readable names for all supported domains, what makes debugging
much easier.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog:
v2:
- sorted all domains data by domain base address in the arrays
---
 drivers/soc/samsung/pm_domains.c | 125 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 123 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 5a0a46bcbe18..43003318b948 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -30,6 +30,17 @@ struct exynos_pm_domain_config {
 	u32 local_pwr_cfg;
 };
 
+struct exynos_pm_domain_data {
+	const char *name;
+	u32 base;
+};
+
+struct exynos_pm_domain_soc_data {
+	const char *compatible;
+	unsigned int nr_domains;
+	const struct exynos_pm_domain_data *domains;
+};
+
 /*
  * Exynos specific wrapper around the generic power domain
  */
@@ -123,6 +134,91 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	return exynos_pd_power(domain, false);
 }
 
+static const struct exynos_pm_domain_data exynos4210_domains[] __initconst = {
+	{ "LCD1",	0x10023CA0 },
+};
+
+static const struct exynos_pm_domain_data exynos4412_domains[] __initconst = {
+	{ "CAM",	0x10023C00 },
+	{ "TV",		0x10023C20 },
+	{ "MFC",	0x10023C40 },
+	{ "G3D",	0x10023C60 },
+	{ "LCD0",	0x10023C80 },
+	{ "ISP",	0x10023CA0 },
+	{ "GPS",	0x10023CE0 },
+	{ "GPS alive",	0x10023D00 },
+};
+
+static const struct exynos_pm_domain_data exynos5250_domains[] __initconst = {
+	{ "GSCL",	0x10044000 },
+	{ "ISP",	0x10044020 },
+	{ "MFC",	0x10044040 },
+	{ "G3D",	0x10044060 },
+	{ "DISP1",	0x100440A0 },
+	{ "MAU",	0x100440C0 },
+};
+
+static const struct exynos_pm_domain_data exynos542x_domains[] __initconst = {
+	{ "SCALER",	0x10044000 },
+	{ "ISP",	0x10044020 },
+	{ "MFC",	0x10044060 },
+	{ "G3D",	0x10044080 },
+	{ "DISP1",	0x100440C0 },
+	{ "MAU",	0x100440E0 },
+	{ "G2D",	0x10044100 },
+	{ "MSCL",	0x10044120 },
+	{ "FSYS",	0x10044140 },
+	{ "PERIC",	0x100441A0 },
+	{ "CAM",	0x10045100 },
+};
+
+static const struct exynos_pm_domain_data exynos5433_domains[] __initconst = {
+	{ "GSCL",	0x105c4000 },
+	{ "CAM0",	0x105c4020 },
+	{ "MSCL",	0x105c4040 },
+	{ "G3D",	0x105c4060 },
+	{ "DISP",	0x105c4080 },
+	{ "CAM1",	0x105c40a0 },
+	{ "AUD",	0x105c40c0 },
+	{ "FSYS",	0x105c40e0 },
+	{ "G2D",	0x105c4120 },
+	{ "ISP",	0x105c4140 },
+	{ "MFC",	0x105c4180 },
+	{ "HEVC",	0x105c41c0 },
+};
+
+static const struct exynos_pm_domain_soc_data soc_domains_data[] __initconst = {
+	{ /* Exynos3250 uses a subset of 4412 domains */
+		.compatible = "samsung,exynos3250",
+		.nr_domains = ARRAY_SIZE(exynos4412_domains),
+		.domains = exynos4412_domains,
+	}, { /* first check samsung,exynos4210 to detect LCD1 domain */
+		.compatible = "samsung,exynos4210",
+		.nr_domains = ARRAY_SIZE(exynos4210_domains),
+		.domains = exynos4210_domains,
+	}, { /* remaining domains for Exynos4210 and 4412 */
+		.compatible = "samsung,exynos4",
+		.nr_domains = ARRAY_SIZE(exynos4412_domains),
+		.domains = exynos4412_domains
+	}, {
+		.compatible = "samsung,exynos5250",
+		.nr_domains = ARRAY_SIZE(exynos5250_domains),
+		.domains = exynos5250_domains,
+	}, {
+		.compatible = "samsung,exynos5420",
+		.nr_domains = ARRAY_SIZE(exynos542x_domains),
+		.domains = exynos542x_domains,
+	}, {
+		.compatible = "samsung,exynos5800",
+		.nr_domains = ARRAY_SIZE(exynos542x_domains),
+		.domains = exynos542x_domains,
+	}, {
+		.compatible = "samsung,exynos5433",
+		.nr_domains = ARRAY_SIZE(exynos5433_domains),
+		.domains = exynos5433_domains,
+	},
+};
+
 static const struct exynos_pm_domain_config exynos4210_cfg __initconst = {
 	.local_pwr_cfg		= 0x7,
 };
@@ -142,6 +238,32 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	{ },
 };
 
+static __init const char *exynos_get_domain_name(struct device_node *np)
+{
+	const struct exynos_pm_domain_soc_data *soc = soc_domains_data;
+	const __be32 *reg;
+	u64 addr;
+	int i, j;
+
+
+	reg = of_get_property(np, "reg", NULL);
+	if (!reg || (addr = of_translate_address(np, reg)) == OF_BAD_ADDR)
+		goto not_found;
+
+	for (i = 0; i < ARRAY_SIZE(soc_domains_data); i++, soc++) {
+		if (!of_machine_is_compatible(soc->compatible))
+			continue;
+
+		for (j = 0; j < soc->nr_domains; j++) {
+			if (soc->domains[j].base == addr)
+				return kstrdup_const(soc->domains[j].name,
+						     GFP_KERNEL);
+		}
+	}
+not_found:
+	return kstrdup_const(strrchr(np->full_name, '/') + 1, GFP_KERNEL);
+}
+
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct device_node *np;
@@ -159,8 +281,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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] soc: samsung: pm_domains: Provide real name for all supported domains
  2017-01-27 14:48         ` [PATCH v2 " Marek Szyprowski
@ 2017-01-28 15:16           ` Krzysztof Kozlowski
  2017-01-30 10:41             ` Marek Szyprowski
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2017-01-28 15:16 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-pm, Sylwester Nawrocki,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

On Fri, Jan 27, 2017 at 03:48:58PM +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 human readable names for all supported domains, what makes debugging
> much easier.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> Changelog:
> v2:
> - sorted all domains data by domain base address in the arrays
> ---
>  drivers/soc/samsung/pm_domains.c | 125 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 123 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
> index 5a0a46bcbe18..43003318b948 100644
> --- a/drivers/soc/samsung/pm_domains.c
> +++ b/drivers/soc/samsung/pm_domains.c
> @@ -30,6 +30,17 @@ struct exynos_pm_domain_config {
>  	u32 local_pwr_cfg;
>  };
>  
> +struct exynos_pm_domain_data {
> +	const char *name;
> +	u32 base;
> +};
> +
> +struct exynos_pm_domain_soc_data {
> +	const char *compatible;
> +	unsigned int nr_domains;
> +	const struct exynos_pm_domain_data *domains;
> +};
> +
>  /*
>   * Exynos specific wrapper around the generic power domain
>   */
> @@ -123,6 +134,91 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>  	return exynos_pd_power(domain, false);
>  }
>  
> +static const struct exynos_pm_domain_data exynos4210_domains[] __initconst = {
> +	{ "LCD1",	0x10023CA0 },
> +};
> +
> +static const struct exynos_pm_domain_data exynos4412_domains[] __initconst = {
> +	{ "CAM",	0x10023C00 },
> +	{ "TV",		0x10023C20 },
> +	{ "MFC",	0x10023C40 },
> +	{ "G3D",	0x10023C60 },
> +	{ "LCD0",	0x10023C80 },
> +	{ "ISP",	0x10023CA0 },
> +	{ "GPS",	0x10023CE0 },
> +	{ "GPS alive",	0x10023D00 },

That is a kind of duplication of DT and also spreading the description
of hardware between DT and driver. I understand the purpose. Messages
like:
	"power-domain has as child subdomain: power-domain."
are useless. However I do not like putting any of these in the driver.

How about adding a property "label" and parse it? Some other
drivers/nodes are using labels already to have a meaningful name, either
for user-space or for just user.

Patches 1-3 look fine.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] soc: samsung: pm_domains: Provide real name for all supported domains
  2017-01-28 15:16           ` Krzysztof Kozlowski
@ 2017-01-30 10:41             ` Marek Szyprowski
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Szyprowski @ 2017-01-30 10:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, linux-pm, Sylwester Nawrocki,
	Javier Martinez Canillas, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Inki Dae

Hi Krzysztof,

On 2017-01-28 16:16, Krzysztof Kozlowski wrote:
> On Fri, Jan 27, 2017 at 03:48:58PM +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 human readable names for all supported domains, what makes debugging
>> much easier.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> Changelog:
>> v2:
>> - sorted all domains data by domain base address in the arrays
>> ---
>>   drivers/soc/samsung/pm_domains.c | 125 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 123 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
>> index 5a0a46bcbe18..43003318b948 100644
>> --- a/drivers/soc/samsung/pm_domains.c
>> +++ b/drivers/soc/samsung/pm_domains.c
>> @@ -30,6 +30,17 @@ struct exynos_pm_domain_config {
>>   	u32 local_pwr_cfg;
>>   };
>>   
>> +struct exynos_pm_domain_data {
>> +	const char *name;
>> +	u32 base;
>> +};
>> +
>> +struct exynos_pm_domain_soc_data {
>> +	const char *compatible;
>> +	unsigned int nr_domains;
>> +	const struct exynos_pm_domain_data *domains;
>> +};
>> +
>>   /*
>>    * Exynos specific wrapper around the generic power domain
>>    */
>> @@ -123,6 +134,91 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>>   	return exynos_pd_power(domain, false);
>>   }
>>   
>> +static const struct exynos_pm_domain_data exynos4210_domains[] __initconst = {
>> +	{ "LCD1",	0x10023CA0 },
>> +};
>> +
>> +static const struct exynos_pm_domain_data exynos4412_domains[] __initconst = {
>> +	{ "CAM",	0x10023C00 },
>> +	{ "TV",		0x10023C20 },
>> +	{ "MFC",	0x10023C40 },
>> +	{ "G3D",	0x10023C60 },
>> +	{ "LCD0",	0x10023C80 },
>> +	{ "ISP",	0x10023CA0 },
>> +	{ "GPS",	0x10023CE0 },
>> +	{ "GPS alive",	0x10023D00 },
> That is a kind of duplication of DT and also spreading the description
> of hardware between DT and driver. I understand the purpose. Messages
> like:
> 	"power-domain has as child subdomain: power-domain."
> are useless. However I do not like putting any of these in the driver.
>
> How about adding a property "label" and parse it? Some other
> drivers/nodes are using labels already to have a meaningful name, either
> for user-space or for just user.
>
> Patches 1-3 look fine.

Okay, I will prepare a new patchset, which rely on 'label' property.

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-01-30 10:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170127104801eucas1p2ca06f8ba0b44e41c0f7dbf0c571bb9a3@eucas1p2.samsung.com>
2017-01-27 10:47 ` [PATCH 0/4] Provide real domain names for Exynos power domain driver Marek Szyprowski
     [not found]   ` <CGME20170127104801eucas1p19a61c607387308a59bdf53fac3efba44@eucas1p1.samsung.com>
2017-01-27 10:47     ` [PATCH 1/4] soc: samsung: pm_domains: Use full names in subdomains registration log Marek Szyprowski
     [not found]   ` <CGME20170127104801eucas1p2287962bae474ca786644de4ad6a11f57@eucas1p2.samsung.com>
2017-01-27 10:47     ` [PATCH 2/4] soc: samsung: pm_domains: Remove unused name field Marek Szyprowski
     [not found]   ` <CGME20170127104802eucas1p27facdbb5c2914da740f4d2ea6b87680b@eucas1p2.samsung.com>
2017-01-27 10:47     ` [PATCH 3/4] soc: samsung: pm_domains: Remove message about failed memory allocation Marek Szyprowski
     [not found]   ` <CGME20170127104802eucas1p121f5f44f8a047ad9b5118a37f66ea032@eucas1p1.samsung.com>
2017-01-27 10:47     ` [PATCH 4/4] soc: samsung: pm_domains: Provide real name for all supported domains Marek Szyprowski
     [not found]       ` <CGME20170127144908eucas1p14c50396f670a01c31a69ec95b636a0e1@eucas1p1.samsung.com>
2017-01-27 14:48         ` [PATCH v2 " Marek Szyprowski
2017-01-28 15:16           ` Krzysztof Kozlowski
2017-01-30 10:41             ` Marek Szyprowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.