All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Exynos5: cleanup clocks handling in power domains
       [not found] <CGME20180221101532eucas1p24200cb65dba8209cd99dba5427c9a28d@eucas1p2.samsung.com>
@ 2018-02-21 10:15 ` Marek Szyprowski
       [not found]   ` <CGME20180221101533eucas1p234b1801844ce8fac633377d129323422@eucas1p2.samsung.com>
                     ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Hello,

This patchset performs cleanup of the clock handling during Exynos power
domain power on/off sequences. This has been achieved by moving all clock
related operations from Exynos power domain driver to respective Exynos
clock controller drivers. Such change is possible after introducinng
runtime power-management in common clock framework.

Another nice result of this cleanup is removal of deplock warning reported
in the following thread (the 'second issue'):
https://www.spinics.net/lists/linux-samsung-soc/msg61766.html

[    5.932966] ======================================================
[    5.937199] usb 5-1: new high-speed USB device number 2 using xhci-hcd
[    5.939073] WARNING: possible circular locking dependency detected
[    5.939110] 4.15.0-rc8-next-20180116 #1121 Tainted: G        W       
[    5.958143] ------------------------------------------------------
[    5.964299] kworker/0:1/59 is trying to acquire lock:
[    5.969304]  (&genpd->mlock){+.+.}, at: [<6abc3872>] genpd_runtime_resume+0x104/0x260
[    5.977155] 
[    5.977155] but task is already holding lock:
[    5.982926]  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
[    5.990143] 
[    5.990143] which lock already depends on the new lock.
[    5.990143] 
[    5.998309] 
[    5.998309] the existing dependency chain (in reverse order) is:
[    6.005739] 
[    6.005739] -> #1 (prepare_lock){+.+.}:
[    6.011042]        mutex_lock_nested+0x1c/0x24
[    6.015419]        clk_prepare_lock+0x50/0xf8
[    6.019755]        clk_unprepare+0x1c/0x2c
[    6.023841]        exynos_pd_power+0x1a8/0x1e4
[    6.028246]        genpd_power_off+0x160/0x274
[    6.032664]        genpd_power_off_work_fn+0x2c/0x40
[    6.037630]        process_one_work+0x2d4/0x8f0
[    6.042104]        worker_thread+0x38/0x584
[    6.046268]        kthread+0x138/0x168
[    6.049981]        ret_from_fork+0x14/0x20
[    6.054044]          (null)
[    6.056794] 
[    6.056794] -> #0 (&genpd->mlock){+.+.}:
[    6.062238]        __mutex_lock+0x7c/0xa68
[    6.066278]        mutex_lock_nested+0x1c/0x24
[    6.070703]        genpd_runtime_resume+0x104/0x260
[    6.075557]        __rpm_callback+0xc0/0x21c
[    6.079792]        rpm_callback+0x20/0x80
[    6.083774]        rpm_resume+0x558/0x7dc
[    6.087762]        __pm_runtime_resume+0x60/0x98
[    6.092367]        clk_core_prepare+0x44/0x490
[    6.096783]        clk_prepare+0x20/0x30
[    6.100674]        amba_get_enable_pclk+0x2c/0x60
[    6.105363]        amba_device_try_add+0x8c/0x20c
[    6.110041]        amba_deferred_retry_func+0x40/0xbc
[    6.115080]        process_one_work+0x2d4/0x8f0
[    6.119569]        worker_thread+0x38/0x584
[    6.123727]        kthread+0x138/0x168
[    6.127444]        ret_from_fork+0x14/0x20
[    6.131510]          (null)
[    6.134263] 
[    6.134263] other info that might help us debug this:
[    6.134263] 
[    6.142328]  Possible unsafe locking scenario:
[    6.142328] 
[    6.148178]        CPU0                    CPU1
[    6.152656]        ----                    ----
[    6.157160]   lock(prepare_lock);
[    6.160439]                                lock(&genpd->mlock);
[    6.166365]                                lock(prepare_lock);
[    6.172168]   lock(&genpd->mlock);
[    6.175517] 
[    6.175517]  *** DEADLOCK ***
[    6.175517] 
[    6.181475] 4 locks held by kworker/0:1/59:
[    6.185580]  #0:  ((wq_completion)"events"){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
[    6.194407]  #1:  ((deferred_retry_work).work){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
[    6.203422]  #2:  (deferred_devices_lock){+.+.}, at: [<3e940c1f>] amba_deferred_retry_func+0x1c/0xbc
[    6.212522]  #3:  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
[    6.220128] 
[    6.220128] stack backtrace:
[    6.224438] CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G        W        4.15.0-rc8-next-20180116 #1121
[    6.233757] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    6.239791] Workqueue: events amba_deferred_retry_func
[    6.244929] [<c0111f90>] (unwind_backtrace) from [<c010e360>] (show_stack+0x10/0x14)
[    6.252670] [<c010e360>] (show_stack) from [<c0a19860>] (dump_stack+0x98/0xc4)
[    6.259877] [<c0a19860>] (dump_stack) from [<c0181478>] (print_circular_bug.constprop.17+0x210/0x32c)
[    6.269077] [<c0181478>] (print_circular_bug.constprop.17) from [<c01848f8>] (__lock_acquire+0x155c/0x1ac8)
[    6.278786] [<c01848f8>] (__lock_acquire) from [<c0185884>] (lock_acquire+0xe0/0x2bc)
[    6.286558] [<c0185884>] (lock_acquire) from [<c0a31788>] (__mutex_lock+0x7c/0xa68)
[    6.294181] [<c0a31788>] (__mutex_lock) from [<c0a32190>] (mutex_lock_nested+0x1c/0x24)
[    6.302161] [<c0a32190>] (mutex_lock_nested) from [<c05885dc>] (genpd_runtime_resume+0x104/0x260)
[    6.311008] [<c05885dc>] (genpd_runtime_resume) from [<c057c6c4>] (__rpm_callback+0xc0/0x21c)
[    6.319484] [<c057c6c4>] (__rpm_callback) from [<c057c840>] (rpm_callback+0x20/0x80)
[    6.327185] [<c057c840>] (rpm_callback) from [<c057c2a8>] (rpm_resume+0x558/0x7dc)
[    6.334721] [<c057c2a8>] (rpm_resume) from [<c057c58c>] (__pm_runtime_resume+0x60/0x98)
[    6.342706] [<c057c58c>] (__pm_runtime_resume) from [<c04b7f6c>] (clk_core_prepare+0x44/0x490)
[    6.351297] [<c04b7f6c>] (clk_core_prepare) from [<c04ba304>] (clk_prepare+0x20/0x30)
[    6.359081] [<c04ba304>] (clk_prepare) from [<c04b52f4>] (amba_get_enable_pclk+0x2c/0x60)
[    6.367229] [<c04b52f4>] (amba_get_enable_pclk) from [<c04b5510>] (amba_device_try_add+0x8c/0x20c)
[    6.376164] [<c04b5510>] (amba_device_try_add) from [<c04b56f8>] (amba_deferred_retry_func+0x40/0xbc)
[    6.385361] [<c04b56f8>] (amba_deferred_retry_func) from [<c01465b4>] (process_one_work+0x2d4/0x8f0)
[    6.394457] [<c01465b4>] (process_one_work) from [<c0147860>] (worker_thread+0x38/0x584)
[    6.402497] [<c0147860>] (worker_thread) from [<c014d794>] (kthread+0x138/0x168)
[    6.409852] [<c014d794>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)

This patchset affects Exynos5250 and Exynos5420/5422/5800 SoCs.

Changes has been tested on Snow Chromebook (Exynos5250), Peach-Pit
Chromebook2 (Exynos5420) and Odroid XU3/XU4 (Exynos5422) boards with
latest linux-next kernel.

If possible I would like to get patches 1-4 merged to current clk-next
tree and patches 5-6 queued to next merge cycle.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (6):
  soc: samsung: pm_domains: Add blacklisting clock handling
  clk: samsung: Add Exynos5x sub-CMU clock driver
  clk: samsung: exynos542x: Move PD-dependent clocks to Exynos5x sub-CMU
    driver
  clk: samsung: exynos5250: Move PD-dependent clocks to Exynos5x sub-CMU
    driver
  soc: samsung: pm_domains: Deprecate support for clocks
  ARM: dts: exynos: Remove obsolete clock properties from power domains

 .../devicetree/bindings/power/pd-samsung.txt       |  20 +--
 arch/arm/boot/dts/exynos5250.dtsi                  |   4 -
 arch/arm/boot/dts/exynos5420.dtsi                  |  14 --
 drivers/clk/samsung/Makefile                       |   2 +
 drivers/clk/samsung/clk-exynos5250.c               |  51 ++++--
 drivers/clk/samsung/clk-exynos5420.c               | 121 +++++++++++---
 drivers/clk/samsung/clk-exynos5x-subcmu.c          | 183 +++++++++++++++++++++
 drivers/clk/samsung/clk-exynos5x-subcmu.h          |  30 ++++
 drivers/soc/samsung/pm_domains.c                   |  79 +--------
 9 files changed, 350 insertions(+), 154 deletions(-)
 create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c
 create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h

-- 
2.15.0


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

* [PATCH 1/6] soc: samsung: pm_domains: Add blacklisting clock handling
       [not found]   ` <CGME20180221101533eucas1p234b1801844ce8fac633377d129323422@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  2018-02-22 10:54       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Handling of clock reparenting will be move to clock controller driver, so
add possibility to blacklist clock handling on systems, where the clock
controller already does all needed operations. This is needed to avoid
potential deadlock on clock reparenting during power domain on/off
procedure.

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

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index b6a436594a19..cef30bdf19b1 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -147,6 +147,9 @@ static __init const char *exynos_get_domain_name(struct device_node *node)
 	return kstrdup_const(name, GFP_KERNEL);
 }
 
+static const char *soc_force_no_clk[] = {
+};
+
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct device_node *np;
@@ -183,6 +186,11 @@ static __init int exynos4_pm_init_power_domain(void)
 		pd->pd.power_on = exynos_pd_power_on;
 		pd->local_pwr_cfg = pm_domain_cfg->local_pwr_cfg;
 
+		for (i = 0; i < ARRAY_SIZE(soc_force_no_clk); i++)
+			if (of_find_compatible_node(NULL, NULL,
+						    soc_force_no_clk[i]))
+				goto no_clk;
+
 		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
 			char clk_name[8];
 
-- 
2.15.0


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

* [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver
       [not found]   ` <CGME20180221101533eucas1p27bcfd237d2c851402b3e99248fec5a6c@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  2018-02-21 16:19       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Exynos5250/5420/5800 have only one clock constroller, but some of their
clock depends on respective power domains. Handling integration of clock
controller and power domain can be done using runtime PM feature of CCF
framework. This however needs a separate struct device for each power
domain. This patch adds such separate driver for group such clocks,
which can be instantiated more than once, each time for a different
power domain.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos5x-subcmu.c | 180 ++++++++++++++++++++++++++++++
 drivers/clk/samsung/clk-exynos5x-subcmu.h |  30 +++++
 2 files changed, 210 insertions(+)
 create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c
 create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h

diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c b/drivers/clk/samsung/clk-exynos5x-subcmu.c
new file mode 100644
index 000000000000..9ff6d5d17f57
--- /dev/null
+++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
@@ -0,0 +1,180 @@
+/*
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Common Clock Framework support for Exynos5x power-domain dependent
+ * sub-CMUs
+ */
+
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+
+#include "clk.h"
+#include "clk-exynos5x-subcmu.h"
+
+static struct samsung_clk_provider *ctx;
+static const struct samsung_5x_subcmu_info *cmu;
+static int nr_cmus;
+
+static void samsung_ext_clk_save(void __iomem *base,
+				    struct samsung_clk_ext_reg_dump *rd,
+				    unsigned int num_regs)
+{
+	for (; num_regs > 0; --num_regs, ++rd) {
+		rd->save = readl(base + rd->offset);
+		writel((rd->save & ~rd->mask) | rd->value, base + rd->offset);
+		rd->save &= rd->mask;
+	}
+};
+
+static void samsung_ext_clk_restore(void __iomem *base,
+				    struct samsung_clk_ext_reg_dump *rd,
+				    unsigned int num_regs)
+{
+	for (; num_regs > 0; --num_regs, ++rd)
+		writel((readl(base + rd->offset) & ~rd->mask) | rd->save,
+		       base + rd->offset);
+}
+
+static int __maybe_unused exynos5x_clk_subcmu_suspend(struct device *dev)
+{
+	struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctx->lock, flags);
+	samsung_ext_clk_save(ctx->reg_base, info->suspend_regs,
+			     info->nr_suspend_regs);
+	spin_unlock_irqrestore(&ctx->lock, flags);
+
+	return 0;
+}
+
+static int __maybe_unused exynos5x_clk_subcmu_resume(struct device *dev)
+{
+	struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctx->lock, flags);
+	samsung_ext_clk_restore(ctx->reg_base, info->suspend_regs,
+				info->nr_suspend_regs);
+	spin_unlock_irqrestore(&ctx->lock, flags);
+
+	return 0;
+}
+
+static void samsung_clk_defer_gate(struct samsung_clk_provider *ctx,
+			  const struct samsung_gate_clock *list, int nr_clk)
+{
+	while (nr_clk--)
+		samsung_clk_add_lookup(ctx, ERR_PTR(-EPROBE_DEFER), list++->id);
+}
+
+void samsung_clk_subcmus_init(struct samsung_clk_provider *_ctx, int _nr_cmus,
+			      const struct samsung_5x_subcmu_info *_cmu)
+{
+	ctx = _ctx;
+	cmu = _cmu;
+	nr_cmus = _nr_cmus;
+
+	for (; _nr_cmus--; _cmu++) {
+		samsung_clk_defer_gate(ctx, _cmu->gate_clks, _cmu->nr_gate_clks);
+		samsung_ext_clk_save(ctx->reg_base, _cmu->suspend_regs,
+				     _cmu->nr_suspend_regs);
+	}
+}
+
+static int __init exynos5x_clk_subcmu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
+
+	pm_runtime_set_suspended(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_get(dev);
+
+	ctx->dev = dev;
+	samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
+	samsung_clk_register_gate(ctx, info->gate_clks, info->nr_gate_clks);
+	ctx->dev = NULL;
+
+	pm_runtime_put_sync(dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops exynos5x_disp_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos5x_clk_subcmu_suspend,
+			   exynos5x_clk_subcmu_resume, NULL)
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				     pm_runtime_force_resume)
+};
+
+static struct platform_driver exynos5x_clk_subcmu_driver __refdata = {
+	.driver	= {
+		.name = "exynos5x-clock-subcmu",
+		.suppress_bind_attrs = true,
+		.pm = &exynos5x_disp_pm_ops,
+	},
+	.probe = exynos5x_clk_subcmu_probe,
+};
+
+static int __init exynos_5x_clk_register_subcmu(struct device *parent,
+					    const struct samsung_5x_subcmu_info *info,
+						struct device_node *pd_node)
+{
+	struct of_phandle_args genpdspec = { .np = pd_node };
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(info->pd_name, -1);
+	pdev->dev.parent = parent;
+	pdev->driver_override = "exynos5x-clock-subcmu";
+	platform_set_drvdata(pdev, (void *)info);
+	of_genpd_add_device(&genpdspec, &pdev->dev);
+	platform_device_add(pdev);
+
+	return 0;
+}
+
+static int __init exynos5x_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np;
+	const char *name;
+	int i;
+
+	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
+		if (of_property_read_string(np, "label", &name) < 0)
+			continue;
+		for (i = 0; i < nr_cmus; i++)
+			if (strcmp(cmu[i].pd_name, name) == 0)
+				exynos_5x_clk_register_subcmu(&pdev->dev,
+							      &cmu[i], np);
+	}
+	return 0;
+}
+
+static const struct of_device_id exynos5x_clk_of_match[] = {
+	{ },
+};
+
+static struct platform_driver exynos5x_clk_driver __refdata = {
+	.driver	= {
+		.name = "exynos5x-clock",
+		.of_match_table = exynos5x_clk_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = exynos5x_clk_probe,
+};
+
+static int __init exynos5x_clk_drv_init(void)
+{
+	platform_driver_register(&exynos5x_clk_driver);
+	platform_driver_register(&exynos5x_clk_subcmu_driver);
+	return 0;
+}
+core_initcall(exynos5x_clk_drv_init);
diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.h b/drivers/clk/samsung/clk-exynos5x-subcmu.h
new file mode 100644
index 000000000000..b44c238e54fa
--- /dev/null
+++ b/drivers/clk/samsung/clk-exynos5x-subcmu.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Common Clock Framework support for Exynos5x power-domain dependent
+ * sub-CMUs
+*/
+
+struct samsung_clk_ext_reg_dump {
+	u32 offset;
+	u32 value;
+	u32 mask;
+	u32 save;
+};
+
+struct samsung_5x_subcmu_info {
+	const struct samsung_div_clock *div_clks;
+	unsigned int nr_div_clks;
+	const struct samsung_gate_clock *gate_clks;
+	unsigned int nr_gate_clks;
+	struct samsung_clk_ext_reg_dump *suspend_regs;
+	unsigned int nr_suspend_regs;
+	const char *pd_name;
+};
+
+void samsung_clk_subcmus_init(struct samsung_clk_provider *ctx, int nr_cmus,
+			      const struct samsung_5x_subcmu_info *cmu);
-- 
2.15.0


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

* [PATCH 3/6] clk: samsung: exynos542x: Move PD-dependent clocks to Exynos5x sub-CMU driver
       [not found]   ` <CGME20180221101533eucas1p2c0fdc0b744b1e026906bd047507f5701@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  2018-02-21 16:21       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Clocks related to DISP, GSC and MFC blocks require special handling for
power domain turn on/off sequences. Till now this was handled by Exynos
power domain driver, but that approach was limited only to some special
cases. This patch moves handling of those operations to clock controller
driver. This gives more flexibility and allows fine tune values of some
clock-specific registers. This patch moves handling of those mentioned
clocks to Exynos5x sub-CMU driver instantiated from Exynos5420 driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/Makefile              |   1 +
 drivers/clk/samsung/clk-exynos5420.c      | 121 +++++++++++++++++++++++-------
 drivers/clk/samsung/clk-exynos5x-subcmu.c |   2 +
 drivers/soc/samsung/pm_domains.c          |   2 +
 4 files changed, 100 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index ef8900bc077f..f70b3f66be89 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
 obj-$(CONFIG_SOC_EXYNOS5260)	+= clk-exynos5260.o
 obj-$(CONFIG_SOC_EXYNOS5410)	+= clk-exynos5410.o
 obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
+obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5x-subcmu.o
 obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos5433.o
 obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
 obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-exynos-audss.o
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 45d34f601e9e..dbf4b5243987 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -19,6 +19,7 @@
 
 #include "clk.h"
 #include "clk-cpu.h"
+#include "clk-exynos5x-subcmu.h"
 
 #define APLL_LOCK		0x0
 #define APLL_CON0		0x100
@@ -863,7 +864,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
 	DIV(0, "dout_mipi1", "mout_mipi1", DIV_DISP10, 16, 8),
 	DIV(0, "dout_dp1", "mout_dp1", DIV_DISP10, 24, 4),
 	DIV(CLK_DOUT_PIXEL, "dout_hdmi_pixel", "mout_pixel", DIV_DISP10, 28, 4),
-	DIV(0, "dout_disp1_blk", "aclk200_disp1", DIV2_RATIO0, 16, 2),
 	DIV(CLK_DOUT_ACLK400_DISP1, "dout_aclk400_disp1",
 			"mout_aclk400_disp1", DIV_TOP2, 4, 3),
 
@@ -912,8 +912,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
 	DIV(0, "dout_spi1", "mout_spi1", DIV_PERIC1, 24, 4),
 	DIV(0, "dout_spi2", "mout_spi2", DIV_PERIC1, 28, 4),
 
-	/* Mfc Block */
-	DIV(0, "dout_mfc_blk", "mout_user_aclk333", DIV4_RATIO, 0, 2),
 
 	/* PCM */
 	DIV(0, "dout_pcm1", "dout_audio1", DIV_PERIC2, 16, 8),
@@ -932,8 +930,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
 	DIV(0, "dout_spi2_pre", "dout_spi2", DIV_PERIC4, 24, 8),
 
 	/* GSCL Block */
-	DIV(0, "dout_gscl_blk_300", "mout_user_aclk300_gscl",
-			DIV2_RATIO0, 4, 2),
 	DIV(0, "dout_gscl_blk_333", "aclk333_432_gscl", DIV2_RATIO0, 6, 2),
 
 	/* MSCL Block */
@@ -1190,8 +1186,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 	GATE(CLK_SCLK_GSCL_WB, "sclk_gscl_wb", "mout_user_aclk333_432_gscl",
 			GATE_TOP_SCLK_GSCL, 7, 0, 0),
 
-	GATE(CLK_GSCL0, "gscl0", "aclk300_gscl", GATE_IP_GSCL0, 0, 0, 0),
-	GATE(CLK_GSCL1, "gscl1", "aclk300_gscl", GATE_IP_GSCL0, 1, 0, 0),
 	GATE(CLK_FIMC_3AA, "fimc_3aa", "aclk333_432_gscl",
 			GATE_IP_GSCL0, 4, 0, 0),
 	GATE(CLK_FIMC_LITE0, "fimc_lite0", "aclk333_432_gscl",
@@ -1205,10 +1199,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 			GATE_IP_GSCL1, 3, 0, 0),
 	GATE(CLK_SMMU_FIMCL1, "smmu_fimcl1", "dout_gscl_blk_333",
 			GATE_IP_GSCL1, 4, 0, 0),
-	GATE(CLK_SMMU_GSCL0, "smmu_gscl0", "dout_gscl_blk_300",
-			GATE_IP_GSCL1, 6, 0, 0),
-	GATE(CLK_SMMU_GSCL1, "smmu_gscl1", "dout_gscl_blk_300",
-			GATE_IP_GSCL1, 7, 0, 0),
 	GATE(CLK_GSCL_WA, "gscl_wa", "sclk_gscl_wa", GATE_IP_GSCL1, 12, 0, 0),
 	GATE(CLK_GSCL_WB, "gscl_wb", "sclk_gscl_wb", GATE_IP_GSCL1, 13, 0, 0),
 	GATE(CLK_SMMU_FIMCL3, "smmu_fimcl3,", "dout_gscl_blk_333",
@@ -1227,18 +1217,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 	GATE(CLK_SMMU_MSCL2, "smmu_mscl2", "dout_mscl_blk",
 			GATE_IP_MSCL, 10, 0, 0),
 
-	GATE(CLK_FIMD1, "fimd1", "aclk300_disp1", GATE_IP_DISP1, 0, 0, 0),
-	GATE(CLK_DSIM1, "dsim1", "aclk200_disp1", GATE_IP_DISP1, 3, 0, 0),
-	GATE(CLK_DP1, "dp1", "aclk200_disp1", GATE_IP_DISP1, 4, 0, 0),
-	GATE(CLK_MIXER, "mixer", "aclk200_disp1", GATE_IP_DISP1, 5, 0, 0),
-	GATE(CLK_HDMI, "hdmi", "aclk200_disp1", GATE_IP_DISP1, 6, 0, 0),
-	GATE(CLK_SMMU_FIMD1M0, "smmu_fimd1m0", "dout_disp1_blk",
-			GATE_IP_DISP1, 7, 0, 0),
-	GATE(CLK_SMMU_FIMD1M1, "smmu_fimd1m1", "dout_disp1_blk",
-			GATE_IP_DISP1, 8, 0, 0),
-	GATE(CLK_SMMU_MIXER, "smmu_mixer", "aclk200_disp1",
-			GATE_IP_DISP1, 9, 0, 0),
-
 	/* ISP */
 	GATE(CLK_SCLK_UART_ISP, "sclk_uart_isp", "dout_uart_isp",
 			GATE_TOP_SCLK_ISP, 0, CLK_SET_RATE_PARENT, 0),
@@ -1255,11 +1233,98 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 	GATE(CLK_SCLK_ISP_SENSOR2, "sclk_isp_sensor2", "dout_isp_sensor2",
 			GATE_TOP_SCLK_ISP, 12, CLK_SET_RATE_PARENT, 0),
 
+	GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9, 0, 0),
+};
+
+static const struct samsung_div_clock exynos5x_disp_div_clks[] __initconst = {
+	DIV(0, "dout_disp1_blk", "aclk200_disp1", DIV2_RATIO0, 16, 2),
+};
+
+static const struct samsung_gate_clock exynos5x_disp_gate_clks[] __initconst = {
+	GATE(CLK_FIMD1, "fimd1", "aclk300_disp1", GATE_IP_DISP1, 0, 0, 0),
+	GATE(CLK_DSIM1, "dsim1", "aclk200_disp1", GATE_IP_DISP1, 3, 0, 0),
+	GATE(CLK_DP1, "dp1", "aclk200_disp1", GATE_IP_DISP1, 4, 0, 0),
+	GATE(CLK_MIXER, "mixer", "aclk200_disp1", GATE_IP_DISP1, 5, 0, 0),
+	GATE(CLK_HDMI, "hdmi", "aclk200_disp1", GATE_IP_DISP1, 6, 0, 0),
+	GATE(CLK_SMMU_FIMD1M0, "smmu_fimd1m0", "dout_disp1_blk",
+			GATE_IP_DISP1, 7, 0, 0),
+	GATE(CLK_SMMU_FIMD1M1, "smmu_fimd1m1", "dout_disp1_blk",
+			GATE_IP_DISP1, 8, 0, 0),
+	GATE(CLK_SMMU_MIXER, "smmu_mixer", "aclk200_disp1",
+			GATE_IP_DISP1, 9, 0, 0),
+};
+
+static struct samsung_clk_ext_reg_dump exynos_5x_disp_suspend_regs[] = {
+	{ GATE_IP_DISP1, 0xffffffff, 0xffffffff }, /* DISP1 gates */
+	{ SRC_TOP5, 0, BIT(0) },	/* MUX mout_user_aclk400_disp1 */
+	{ SRC_TOP5, 0, BIT(24) },	/* MUX mout_user_aclk300_disp1 */
+	{ SRC_TOP3, 0, BIT(8) },	/* MUX mout_user_aclk200_disp1 */
+	{ DIV2_RATIO0, 0, 0x30000 },		/* DIV dout_disp1_blk */
+};
+
+static const struct samsung_div_clock exynos5x_gsc_div_clks[] __initconst = {
+	DIV(0, "dout_gscl_blk_300", "mout_user_aclk300_gscl",
+			DIV2_RATIO0, 4, 2),
+};
+
+static const struct samsung_gate_clock exynos5x_gsc_gate_clks[] __initconst = {
+	GATE(CLK_GSCL0, "gscl0", "aclk300_gscl", GATE_IP_GSCL0, 0, 0, 0),
+	GATE(CLK_GSCL1, "gscl1", "aclk300_gscl", GATE_IP_GSCL0, 1, 0, 0),
+	GATE(CLK_SMMU_GSCL0, "smmu_gscl0", "dout_gscl_blk_300",
+			GATE_IP_GSCL1, 6, 0, 0),
+	GATE(CLK_SMMU_GSCL1, "smmu_gscl1", "dout_gscl_blk_300",
+			GATE_IP_GSCL1, 7, 0, 0),
+};
+
+static struct samsung_clk_ext_reg_dump exynos_5x_gsc_suspend_regs[] = {
+	{ GATE_IP_GSCL0, 0x3, 0x3 },	/* GSC gates */
+	{ GATE_IP_GSCL1, 0xc0, 0xc0 },	/* GSC gates */
+	{ SRC_TOP5, 0, BIT(28) },	/* MUX mout_user_aclk300_gscl */
+	{ DIV2_RATIO0, 0, 0x30 },	/* DIV dout_gscl_blk_300 */
+};
+
+static const struct samsung_div_clock exynos5x_mfc_div_clks[] __initconst = {
+	DIV(0, "dout_mfc_blk", "mout_user_aclk333", DIV4_RATIO, 0, 2),
+};
+
+static const struct samsung_gate_clock exynos5x_mfc_gate_clks[] __initconst = {
 	GATE(CLK_MFC, "mfc", "aclk333", GATE_IP_MFC, 0, 0, 0),
 	GATE(CLK_SMMU_MFCL, "smmu_mfcl", "dout_mfc_blk", GATE_IP_MFC, 1, 0, 0),
 	GATE(CLK_SMMU_MFCR, "smmu_mfcr", "dout_mfc_blk", GATE_IP_MFC, 2, 0, 0),
+};
 
-	GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9, 0, 0),
+static struct samsung_clk_ext_reg_dump exynos_5x_mfc_suspend_regs[] = {
+	{ GATE_IP_MFC, 0xffffffff, 0xffffffff }, /* MFC gates */
+	{ SRC_TOP4, 0, BIT(28) },		/* MUX mout_user_aclk333 */
+	{ DIV4_RATIO, 0, 0x3 },			/* DIV dout_mfc_blk */
+};
+
+static const struct samsung_5x_subcmu_info exynos_5x_subcmus[] = {
+	{
+		.div_clks	= exynos5x_disp_div_clks,
+		.nr_div_clks	= ARRAY_SIZE(exynos5x_disp_div_clks),
+		.gate_clks	= exynos5x_disp_gate_clks,
+		.nr_gate_clks	= ARRAY_SIZE(exynos5x_disp_gate_clks),
+		.suspend_regs	= exynos_5x_disp_suspend_regs,
+		.nr_suspend_regs = ARRAY_SIZE(exynos_5x_disp_suspend_regs),
+		.pd_name	= "DISP",
+	}, {
+		.div_clks	= exynos5x_gsc_div_clks,
+		.nr_div_clks	= ARRAY_SIZE(exynos5x_gsc_div_clks),
+		.gate_clks	= exynos5x_gsc_gate_clks,
+		.nr_gate_clks	= ARRAY_SIZE(exynos5x_gsc_gate_clks),
+		.suspend_regs	= exynos_5x_gsc_suspend_regs,
+		.nr_suspend_regs = ARRAY_SIZE(exynos_5x_gsc_suspend_regs),
+		.pd_name	= "GSC",
+	}, {
+		.div_clks	= exynos5x_mfc_div_clks,
+		.nr_div_clks	= ARRAY_SIZE(exynos5x_mfc_div_clks),
+		.gate_clks	= exynos5x_mfc_gate_clks,
+		.nr_gate_clks	= ARRAY_SIZE(exynos5x_mfc_gate_clks),
+		.suspend_regs	= exynos_5x_mfc_suspend_regs,
+		.nr_suspend_regs = ARRAY_SIZE(exynos_5x_mfc_suspend_regs),
+		.pd_name	= "MFC",
+	},
 };
 
 static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] __initconst = {
@@ -1472,6 +1537,8 @@ static void __init exynos5x_clk_init(struct device_node *np,
 		exynos5420_kfcclk_d, ARRAY_SIZE(exynos5420_kfcclk_d), 0);
 
 	exynos5420_clk_sleep_init();
+	samsung_clk_subcmus_init(ctx, ARRAY_SIZE(exynos_5x_subcmus),
+				 exynos_5x_subcmus);
 
 	samsung_clk_of_add_provider(np, ctx);
 }
@@ -1480,10 +1547,12 @@ static void __init exynos5420_clk_init(struct device_node *np)
 {
 	exynos5x_clk_init(np, EXYNOS5420);
 }
-CLK_OF_DECLARE(exynos5420_clk, "samsung,exynos5420-clock", exynos5420_clk_init);
+CLK_OF_DECLARE_DRIVER(exynos5420_clk, "samsung,exynos5420-clock",
+		      exynos5420_clk_init);
 
 static void __init exynos5800_clk_init(struct device_node *np)
 {
 	exynos5x_clk_init(np, EXYNOS5800);
 }
-CLK_OF_DECLARE(exynos5800_clk, "samsung,exynos5800-clock", exynos5800_clk_init);
+CLK_OF_DECLARE_DRIVER(exynos5800_clk, "samsung,exynos5800-clock",
+		      exynos5800_clk_init);
diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c b/drivers/clk/samsung/clk-exynos5x-subcmu.c
index 9ff6d5d17f57..256473b83264 100644
--- a/drivers/clk/samsung/clk-exynos5x-subcmu.c
+++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
@@ -159,6 +159,8 @@ static int __init exynos5x_clk_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id exynos5x_clk_of_match[] = {
+	{ .compatible = "samsung,exynos5420-clock", },
+	{ .compatible = "samsung,exynos5800-clock", },
 	{ },
 };
 
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index cef30bdf19b1..f2d6d7a09c16 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -148,6 +148,8 @@ static __init const char *exynos_get_domain_name(struct device_node *node)
 }
 
 static const char *soc_force_no_clk[] = {
+	"samsung,exynos5420-clock",
+	"samsung,exynos5800-clock",
 };
 
 static __init int exynos4_pm_init_power_domain(void)
-- 
2.15.0


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

* [PATCH 4/6] clk: samsung: exynos5250: Move PD-dependent clocks to Exynos5x sub-CMU driver
       [not found]   ` <CGME20180221101534eucas1p24db3c1d049d9ecd0de9c76a10bb58041@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  2018-02-22 10:54       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Clocks related to DISP1 block require special handling for power domain
turn on/off sequences. Till now this was handled by Exynos power domain
driver, but that approach was limited only to some special cases. This
patch moves handling of those operations to clock controller driver.
This gives more flexibility and allows fine tune values of some
clock-specific registers. This patch moves handling of those mentioned
clocks to Exynos5x sub-CMU driver instantiated from Exynos5250 driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/Makefile              |  1 +
 drivers/clk/samsung/clk-exynos5250.c      | 51 +++++++++++++++++++++----------
 drivers/clk/samsung/clk-exynos5x-subcmu.c |  1 +
 drivers/soc/samsung/pm_domains.c          |  1 +
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index f70b3f66be89..d265f4babfb0 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_SOC_EXYNOS3250)	+= clk-exynos3250.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4412-isp.o
 obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
+obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5x-subcmu.o
 obj-$(CONFIG_SOC_EXYNOS5260)	+= clk-exynos5260.o
 obj-$(CONFIG_SOC_EXYNOS5410)	+= clk-exynos5410.o
 obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 9b073c98a891..876fa4c122ca 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -18,6 +18,7 @@
 
 #include "clk.h"
 #include "clk-cpu.h"
+#include "clk-exynos5x-subcmu.h"
 
 #define APLL_LOCK		0x0
 #define APLL_CON0		0x100
@@ -571,17 +572,6 @@ static const struct samsung_gate_clock exynos5250_gate_clks[] __initconst = {
 	GATE(CLK_SMMU_GSCL3, "smmu_gscl3", "mout_aclk266_gscl_sub",
 			GATE_IP_GSCL, 10, 0, 0),
 
-	GATE(CLK_FIMD1, "fimd1", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 0, 0,
-		0),
-	GATE(CLK_MIE1, "mie1", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 1, 0,
-		0),
-	GATE(CLK_DSIM0, "dsim0", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 3, 0,
-		0),
-	GATE(CLK_DP, "dp", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 4, 0, 0),
-	GATE(CLK_MIXER, "mixer", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 5, 0,
-		0),
-	GATE(CLK_HDMI, "hdmi", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 6, 0,
-		0),
 
 	GATE(CLK_MFC, "mfc", "mout_aclk333_sub", GATE_IP_MFC, 0, 0, 0),
 	GATE(CLK_SMMU_MFCR, "smmu_mfcr", "mout_aclk333_sub", GATE_IP_MFC, 1, 0,
@@ -671,10 +661,6 @@ static const struct samsung_gate_clock exynos5250_gate_clks[] __initconst = {
 	GATE(CLK_WDT, "wdt", "div_aclk66", GATE_IP_PERIS, 19, 0, 0),
 	GATE(CLK_RTC, "rtc", "div_aclk66", GATE_IP_PERIS, 20, 0, 0),
 	GATE(CLK_TMU, "tmu", "div_aclk66", GATE_IP_PERIS, 21, 0, 0),
-	GATE(CLK_SMMU_TV, "smmu_tv", "mout_aclk200_disp1_sub",
-			GATE_IP_DISP1, 9, 0, 0),
-	GATE(CLK_SMMU_FIMD1, "smmu_fimd1", "mout_aclk200_disp1_sub",
-			GATE_IP_DISP1, 8, 0, 0),
 	GATE(CLK_SMMU_2D, "smmu_2d", "div_aclk200", GATE_IP_ACP, 7, 0, 0),
 	GATE(CLK_SMMU_FIMC_ISP, "smmu_fimc_isp", "mout_aclk_266_isp_sub",
 			GATE_IP_ISP0, 8, 0, 0),
@@ -698,6 +684,38 @@ static const struct samsung_gate_clock exynos5250_gate_clks[] __initconst = {
 			GATE_IP_ISP1, 7, 0, 0),
 };
 
+static const struct samsung_gate_clock exynos5250_disp_gate_clks[] __initconst = {
+	GATE(CLK_FIMD1, "fimd1", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 0, 0,
+		0),
+	GATE(CLK_MIE1, "mie1", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 1, 0,
+		0),
+	GATE(CLK_DSIM0, "dsim0", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 3, 0,
+		0),
+	GATE(CLK_DP, "dp", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 4, 0, 0),
+	GATE(CLK_MIXER, "mixer", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 5, 0,
+		0),
+	GATE(CLK_HDMI, "hdmi", "mout_aclk200_disp1_sub", GATE_IP_DISP1, 6, 0,
+		0),
+	GATE(CLK_SMMU_TV, "smmu_tv", "mout_aclk200_disp1_sub",
+			GATE_IP_DISP1, 9, 0, 0),
+	GATE(CLK_SMMU_FIMD1, "smmu_fimd1", "mout_aclk200_disp1_sub",
+			GATE_IP_DISP1, 8, 0, 0),
+};
+
+static struct samsung_clk_ext_reg_dump exynos_5250_disp_suspend_regs[] = {
+	{ GATE_IP_DISP1, 0xffffffff, 0xffffffff }, /* DISP1 gates */
+	{ SRC_TOP3, 0, BIT(4) },	/* MUX mout_aclk200_disp1_sub */
+	{ SRC_TOP3, 0, BIT(6) },	/* MUX mout_aclk300_disp1_sub */
+};
+
+static const struct samsung_5x_subcmu_info exynos_5250_disp_subcmu = {
+	.gate_clks	= exynos5250_disp_gate_clks,
+	.nr_gate_clks	= ARRAY_SIZE(exynos5250_disp_gate_clks),
+	.suspend_regs	= exynos_5250_disp_suspend_regs,
+	.nr_suspend_regs = ARRAY_SIZE(exynos_5250_disp_suspend_regs),
+	.pd_name	= "DISP1",
+};
+
 static const struct samsung_pll_rate_table vpll_24mhz_tbl[] __initconst = {
 	/* sorted in descending order */
 	/* PLL_36XX_RATE(rate, m, p, s, k) */
@@ -859,10 +877,11 @@ static void __init exynos5250_clk_init(struct device_node *np)
 	__raw_writel(tmp, reg_base + PWR_CTRL2);
 
 	exynos5250_clk_sleep_init();
+	samsung_clk_subcmus_init(ctx, 1, &exynos_5250_disp_subcmu);
 
 	samsung_clk_of_add_provider(np, ctx);
 
 	pr_info("Exynos5250: clock setup completed, armclk=%ld\n",
 			_get_rate("div_arm2"));
 }
-CLK_OF_DECLARE(exynos5250_clk, "samsung,exynos5250-clock", exynos5250_clk_init);
+CLK_OF_DECLARE_DRIVER(exynos5250_clk, "samsung,exynos5250-clock", exynos5250_clk_init);
diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c b/drivers/clk/samsung/clk-exynos5x-subcmu.c
index 256473b83264..483f70c2dd40 100644
--- a/drivers/clk/samsung/clk-exynos5x-subcmu.c
+++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
@@ -159,6 +159,7 @@ static int __init exynos5x_clk_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id exynos5x_clk_of_match[] = {
+	{ .compatible = "samsung,exynos5250-clock", },
 	{ .compatible = "samsung,exynos5420-clock", },
 	{ .compatible = "samsung,exynos5800-clock", },
 	{ },
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index f2d6d7a09c16..caf45cf7aa8e 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -148,6 +148,7 @@ static __init const char *exynos_get_domain_name(struct device_node *node)
 }
 
 static const char *soc_force_no_clk[] = {
+	"samsung,exynos5250-clock",
 	"samsung,exynos5420-clock",
 	"samsung,exynos5800-clock",
 };
-- 
2.15.0


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

* [PATCH 5/6] soc: samsung: pm_domains: Deprecate support for clocks
       [not found]   ` <CGME20180221101534eucas1p29d832ffe11055241a39d79a8863845ad@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Handling of special clock operations on power domain on/off sequences has
been moved to respective Exynos clock controller drivers, so there is no
need to keep the duplicated (and conflicting) code in Exynos power domain
driver. Mark clock related properties in Exynos power domain bindings as
deprecated. This change has no inpact on backwards-compatibility, as the
new drivers properly work with old DTBs (deprecated properties are
ignored).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/power/pd-samsung.txt       | 20 +----
 drivers/soc/samsung/pm_domains.c                   | 90 +---------------------
 2 files changed, 5 insertions(+), 105 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/pd-samsung.txt b/Documentation/devicetree/bindings/power/pd-samsung.txt
index 549f7dee9b9d..92ef355e8f64 100644
--- a/Documentation/devicetree/bindings/power/pd-samsung.txt
+++ b/Documentation/devicetree/bindings/power/pd-samsung.txt
@@ -15,23 +15,13 @@ Required Properties:
 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
-	all domains which are powered on and off and not required for unused
-	domains.
-- clock-names: The following clocks can be specified:
-	- oscclk: Oscillator clock.
-	- clkN: Input clocks to the devices in this power domain. These clocks
-		will be reparented to oscclk before switching power domain off.
-		Their original parent will be brought back after turning on
-		the domain. Maximum of 4 clocks (N = 0 to 3) are supported.
-	- asbN: Clocks required by asynchronous bridges (ASB) present in
-		the power domain. These clock should be enabled during power
-		domain on/off operations.
 - power-domains: phandle pointing to the parent power domain, for more details
 		 see Documentation/devicetree/bindings/power/power_domain.txt
 
+Deprecated Properties:
+- clocks
+- clock-names
+
 Node of a device using power domains must have a power-domains property
 defined with a phandle to respective power domain.
 
@@ -47,8 +37,6 @@ Example:
 	mfc_pd: power-domain@10044060 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10044060 0x20>;
-		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>;
-		clock-names = "oscclk", "clk0";
 		#power-domain-cells = <0>;
 		label = "MFC";
 	};
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index caf45cf7aa8e..ab8582971bfc 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -13,14 +13,11 @@
 #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>
 #include <linux/sched.h>
 
-#define MAX_CLK_PER_DOMAIN	4
-
 struct exynos_pm_domain_config {
 	/* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
 	u32 local_pwr_cfg;
@@ -33,10 +30,6 @@ struct exynos_pm_domain {
 	void __iomem *base;
 	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];
-	struct clk *asb_clk[MAX_CLK_PER_DOMAIN];
 	u32 local_pwr_cfg;
 };
 
@@ -46,29 +39,10 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 	void __iomem *base;
 	u32 timeout, pwr;
 	char *op;
-	int i;
 
 	pd = container_of(domain, struct exynos_pm_domain, pd);
 	base = pd->base;
 
-	for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-		if (IS_ERR(pd->asb_clk[i]))
-			break;
-		clk_prepare_enable(pd->asb_clk[i]);
-	}
-
-	/* Set oscclk before powering off a domain*/
-	if (!power_on) {
-		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-			if (IS_ERR(pd->clk[i]))
-				break;
-			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",
-						domain->name, i);
-		}
-	}
-
 	pwr = power_on ? pd->local_pwr_cfg : 0;
 	writel_relaxed(pwr, base);
 
@@ -86,26 +60,6 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 		usleep_range(80, 100);
 	}
 
-	/* Restore clocks after powering on a domain*/
-	if (power_on) {
-		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-			if (IS_ERR(pd->clk[i]))
-				break;
-
-			if (IS_ERR(pd->pclk[i]))
-				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",
-						domain->name, i);
-		}
-	}
-
-	for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-		if (IS_ERR(pd->asb_clk[i]))
-			break;
-		clk_disable_unprepare(pd->asb_clk[i]);
-	}
-
 	return 0;
 }
 
@@ -147,12 +101,6 @@ static __init const char *exynos_get_domain_name(struct device_node *node)
 	return kstrdup_const(name, GFP_KERNEL);
 }
 
-static const char *soc_force_no_clk[] = {
-	"samsung,exynos5250-clock",
-	"samsung,exynos5420-clock",
-	"samsung,exynos5800-clock",
-};
-
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct device_node *np;
@@ -161,7 +109,7 @@ static __init int exynos4_pm_init_power_domain(void)
 	for_each_matching_node_and_match(np, exynos_pm_domain_of_match, &match) {
 		const struct exynos_pm_domain_config *pm_domain_cfg;
 		struct exynos_pm_domain *pd;
-		int on, i;
+		int on;
 
 		pm_domain_cfg = match->data;
 
@@ -189,42 +137,6 @@ static __init int exynos4_pm_init_power_domain(void)
 		pd->pd.power_on = exynos_pd_power_on;
 		pd->local_pwr_cfg = pm_domain_cfg->local_pwr_cfg;
 
-		for (i = 0; i < ARRAY_SIZE(soc_force_no_clk); i++)
-			if (of_find_compatible_node(NULL, NULL,
-						    soc_force_no_clk[i]))
-				goto no_clk;
-
-		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-			char clk_name[8];
-
-			snprintf(clk_name, sizeof(clk_name), "asb%d", i);
-			pd->asb_clk[i] = of_clk_get_by_name(np, clk_name);
-			if (IS_ERR(pd->asb_clk[i]))
-				break;
-		}
-
-		pd->oscclk = of_clk_get_by_name(np, "oscclk");
-		if (IS_ERR(pd->oscclk))
-			goto no_clk;
-
-		for (i = 0; i < MAX_CLK_PER_DOMAIN; i++) {
-			char clk_name[8];
-
-			snprintf(clk_name, sizeof(clk_name), "clk%d", i);
-			pd->clk[i] = of_clk_get_by_name(np, clk_name);
-			if (IS_ERR(pd->clk[i]))
-				break;
-			/*
-			 * Skip setting parent on first power up.
-			 * The parent at this time may not be useful at all.
-			 */
-			pd->pclk[i] = ERR_PTR(-EINVAL);
-		}
-
-		if (IS_ERR(pd->clk[0]))
-			clk_put(pd->oscclk);
-
-no_clk:
 		on = readl_relaxed(pd->base + 0x4) & pd->local_pwr_cfg;
 
 		pm_genpd_init(&pd->pd, NULL, !on);
-- 
2.15.0


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

* [PATCH 6/6] ARM: dts: exynos: Remove obsolete clock properties from power domains
       [not found]   ` <CGME20180221101535eucas1p2e99fbf00fd97bb935b02d12e7c225da0@eucas1p2.samsung.com>
@ 2018-02-21 10:15     ` Marek Szyprowski
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-21 10:15 UTC (permalink / raw)
  To: linux-clk, linux-samsung-soc
  Cc: Marek Szyprowski, Sylwester Nawrocki, Chanwoo Choi, Inki Dae,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Handling of special clock operations on power domain on/off sequences has
been moved to respective Exynos clock controller drivers and clock properties
have been marked as deprecated. Remove all clock properties from existing
Exynos power domain nodes, as they are no longer used.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |  4 ----
 arch/arm/boot/dts/exynos5420.dtsi | 14 --------------
 2 files changed, 18 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index bb4180ef7885..54b0ecd2c4fd 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -132,10 +132,6 @@
 			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>;
-			clock-names = "oscclk", "clk0", "clk1";
 		};
 
 		pd_mau: power-domain@100440c0 {
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 2f3cb2a97f71..9672d0e51f69 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -276,10 +276,6 @@
 			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>;
-			clock-names = "oscclk", "clk0", "asb0", "asb1";
 		};
 
 		isp_pd: power-domain@10044020 {
@@ -292,10 +288,6 @@
 		mfc_pd: power-domain@10044060 {
 			compatible = "samsung,exynos4210-pd";
 			reg = <0x10044060 0x20>;
-			clocks = <&clock CLK_FIN_PLL>,
-				 <&clock CLK_MOUT_USER_ACLK333>,
-				 <&clock CLK_ACLK333>;
-			clock-names = "oscclk", "clk0","asb0";
 			#power-domain-cells = <0>;
 			label = "MFC";
 		};
@@ -312,12 +304,6 @@
 			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>,
-				 <&clock CLK_MOUT_USER_ACLK400_DISP1>,
-				 <&clock CLK_FIMD1>, <&clock CLK_MIXER>;
-			clock-names = "oscclk", "clk0", "clk1", "clk2", "asb0", "asb1";
 		};
 
 		mau_pd: power-domain@100440e0 {
-- 
2.15.0


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

* Re: [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver
  2018-02-21 10:15     ` [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver Marek Szyprowski
@ 2018-02-21 16:19       ` Krzysztof Kozlowski
  2018-02-22 12:22         ` Marek Szyprowski
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2018-02-21 16:19 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Exynos5250/5420/5800 have only one clock constroller, but some of their
> clock depends on respective power domains. Handling integration of clock
> controller and power domain can be done using runtime PM feature of CCF
> framework. This however needs a separate struct device for each power
> domain. This patch adds such separate driver for group such clocks,

"driver to group"?

> which can be instantiated more than once, each time for a different
> power domain.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5x-subcmu.c | 180 ++++++++++++++++++++++++++++++

In some other places we call entire family "exynos5" so maybe let's
follow the convention instead of "5x"? In the name of file and all the
variables/names later?

>  drivers/clk/samsung/clk-exynos5x-subcmu.h |  30 +++++
>  2 files changed, 210 insertions(+)
>  create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c
>  create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h
>
> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c b/drivers/clk/samsung/clk-exynos5x-subcmu.c
> new file mode 100644
> index 000000000000..9ff6d5d17f57
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

Use SPDX identifier instead of license text.

> + *
> + * Common Clock Framework support for Exynos5x power-domain dependent
> + * sub-CMUs
> + */
> +
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "clk.h"
> +#include "clk-exynos5x-subcmu.h"
> +
> +static struct samsung_clk_provider *ctx;
> +static const struct samsung_5x_subcmu_info *cmu;
> +static int nr_cmus;

core_initcall here will register this and subcmu drivers. The probe of
this driver will populate devices for subcmus. From next patch - the
initcall of clock driver - will initialize the subcmus, also touching
these statics.

Is my understanding correct?

Quite complicated init system... plus static variables. Are you sure
there are no concurrency issues?

> +
> +static void samsung_ext_clk_save(void __iomem *base,
> +                                   struct samsung_clk_ext_reg_dump *rd,
> +                                   unsigned int num_regs)
> +{
> +       for (; num_regs > 0; --num_regs, ++rd) {
> +               rd->save = readl(base + rd->offset);
> +               writel((rd->save & ~rd->mask) | rd->value, base + rd->offset);
> +               rd->save &= rd->mask;
> +       }
> +};
> +
> +static void samsung_ext_clk_restore(void __iomem *base,
> +                                   struct samsung_clk_ext_reg_dump *rd,
> +                                   unsigned int num_regs)
> +{
> +       for (; num_regs > 0; --num_regs, ++rd)
> +               writel((readl(base + rd->offset) & ~rd->mask) | rd->save,
> +                      base + rd->offset);
> +}
> +
> +static int __maybe_unused exynos5x_clk_subcmu_suspend(struct device *dev)
> +{
> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&ctx->lock, flags);
> +       samsung_ext_clk_save(ctx->reg_base, info->suspend_regs,
> +                            info->nr_suspend_regs);
> +       spin_unlock_irqrestore(&ctx->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused exynos5x_clk_subcmu_resume(struct device *dev)
> +{
> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&ctx->lock, flags);
> +       samsung_ext_clk_restore(ctx->reg_base, info->suspend_regs,
> +                               info->nr_suspend_regs);
> +       spin_unlock_irqrestore(&ctx->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void samsung_clk_defer_gate(struct samsung_clk_provider *ctx,
> +                         const struct samsung_gate_clock *list, int nr_clk)
> +{
> +       while (nr_clk--)
> +               samsung_clk_add_lookup(ctx, ERR_PTR(-EPROBE_DEFER), list++->id);
> +}
> +
> +void samsung_clk_subcmus_init(struct samsung_clk_provider *_ctx, int _nr_cmus,
> +                             const struct samsung_5x_subcmu_info *_cmu)
> +{
> +       ctx = _ctx;
> +       cmu = _cmu;
> +       nr_cmus = _nr_cmus;
> +
> +       for (; _nr_cmus--; _cmu++) {
> +               samsung_clk_defer_gate(ctx, _cmu->gate_clks, _cmu->nr_gate_clks);
> +               samsung_ext_clk_save(ctx->reg_base, _cmu->suspend_regs,
> +                                    _cmu->nr_suspend_regs);
> +       }
> +}
> +
> +static int __init exynos5x_clk_subcmu_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
> +
> +       pm_runtime_set_suspended(dev);
> +       pm_runtime_enable(dev);
> +       pm_runtime_get(dev);
> +
> +       ctx->dev = dev;
> +       samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
> +       samsung_clk_register_gate(ctx, info->gate_clks, info->nr_gate_clks);
> +       ctx->dev = NULL;
> +
> +       pm_runtime_put_sync(dev);
> +
> +       return 0;
> +}
> +
> +static const struct dev_pm_ops exynos5x_disp_pm_ops = {
> +       SET_RUNTIME_PM_OPS(exynos5x_clk_subcmu_suspend,
> +                          exynos5x_clk_subcmu_resume, NULL)
> +       SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +                                    pm_runtime_force_resume)
> +};
> +
> +static struct platform_driver exynos5x_clk_subcmu_driver __refdata = {
> +       .driver = {
> +               .name = "exynos5x-clock-subcmu",
> +               .suppress_bind_attrs = true,
> +               .pm = &exynos5x_disp_pm_ops,
> +       },
> +       .probe = exynos5x_clk_subcmu_probe,
> +};
> +
> +static int __init exynos_5x_clk_register_subcmu(struct device *parent,
> +                                           const struct samsung_5x_subcmu_info *info,
> +                                               struct device_node *pd_node)
> +{
> +       struct of_phandle_args genpdspec = { .np = pd_node };
> +       struct platform_device *pdev;
> +
> +       pdev = platform_device_alloc(info->pd_name, -1);
> +       pdev->dev.parent = parent;
> +       pdev->driver_override = "exynos5x-clock-subcmu";
> +       platform_set_drvdata(pdev, (void *)info);
> +       of_genpd_add_device(&genpdspec, &pdev->dev);
> +       platform_device_add(pdev);
> +
> +       return 0;
> +}
> +
> +static int __init exynos5x_clk_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np;
> +       const char *name;
> +       int i;
> +
> +       for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
> +               if (of_property_read_string(np, "label", &name) < 0)
> +                       continue;
> +               for (i = 0; i < nr_cmus; i++)
> +                       if (strcmp(cmu[i].pd_name, name) == 0)
> +                               exynos_5x_clk_register_subcmu(&pdev->dev,
> +                                                             &cmu[i], np);
> +       }
> +       return 0;
> +}
> +
> +static const struct of_device_id exynos5x_clk_of_match[] = {
> +       { },
> +};
> +
> +static struct platform_driver exynos5x_clk_driver __refdata = {
> +       .driver = {
> +               .name = "exynos5x-clock",
> +               .of_match_table = exynos5x_clk_of_match,
> +               .suppress_bind_attrs = true,
> +       },
> +       .probe = exynos5x_clk_probe,
> +};
> +
> +static int __init exynos5x_clk_drv_init(void)
> +{
> +       platform_driver_register(&exynos5x_clk_driver);
> +       platform_driver_register(&exynos5x_clk_subcmu_driver);
> +       return 0;
> +}
> +core_initcall(exynos5x_clk_drv_init);
> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.h b/drivers/clk/samsung/clk-exynos5x-subcmu.h
> new file mode 100644
> index 000000000000..b44c238e54fa
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

SPDX as well plus a include guard.

BR,
Krzysztof

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

* Re: [PATCH 3/6] clk: samsung: exynos542x: Move PD-dependent clocks to Exynos5x sub-CMU driver
  2018-02-21 10:15     ` [PATCH 3/6] clk: samsung: exynos542x: Move PD-dependent clocks to Exynos5x sub-CMU driver Marek Szyprowski
@ 2018-02-21 16:21       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2018-02-21 16:21 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

Minor nit - the driver you are touching is "clk-exynos5420" so let's
use the same prefix for commit subject (instead of 542x).

With the patch itself I am okay:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Clocks related to DISP, GSC and MFC blocks require special handling for
> power domain turn on/off sequences. Till now this was handled by Exynos
> power domain driver, but that approach was limited only to some special
> cases. This patch moves handling of those operations to clock controller
> driver. This gives more flexibility and allows fine tune values of some
> clock-specific registers. This patch moves handling of those mentioned
> clocks to Exynos5x sub-CMU driver instantiated from Exynos5420 driver.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clk/samsung/Makefile              |   1 +
>  drivers/clk/samsung/clk-exynos5420.c      | 121 +++++++++++++++++++++++-------
>  drivers/clk/samsung/clk-exynos5x-subcmu.c |   2 +
>  drivers/soc/samsung/pm_domains.c          |   2 +
>  4 files changed, 100 insertions(+), 26 deletions(-)

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

* Re: [PATCH 1/6] soc: samsung: pm_domains: Add blacklisting clock handling
  2018-02-21 10:15     ` [PATCH 1/6] soc: samsung: pm_domains: Add blacklisting clock handling Marek Szyprowski
@ 2018-02-22 10:54       ` Krzysztof Kozlowski
  2018-03-06 16:05         ` Sylwester Nawrocki
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2018-02-22 10:54 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Handling of clock reparenting will be move to clock controller driver, so
> add possibility to blacklist clock handling on systems, where the clock
> controller already does all needed operations. This is needed to avoid
> potential deadlock on clock reparenting during power domain on/off
> procedure.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/soc/samsung/pm_domains.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Feel free to take it through samsung-clk tree. Just in case of
possible conflicts - could you put it on separate branch please?

Best regards,
Krzysztof

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

* Re: [PATCH 4/6] clk: samsung: exynos5250: Move PD-dependent clocks to Exynos5x sub-CMU driver
  2018-02-21 10:15     ` [PATCH 4/6] clk: samsung: exynos5250: " Marek Szyprowski
@ 2018-02-22 10:54       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2018-02-22 10:54 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Clocks related to DISP1 block require special handling for power domain
> turn on/off sequences. Till now this was handled by Exynos power domain
> driver, but that approach was limited only to some special cases. This
> patch moves handling of those operations to clock controller driver.
> This gives more flexibility and allows fine tune values of some
> clock-specific registers. This patch moves handling of those mentioned
> clocks to Exynos5x sub-CMU driver instantiated from Exynos5250 driver.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clk/samsung/Makefile              |  1 +
>  drivers/clk/samsung/clk-exynos5250.c      | 51 +++++++++++++++++++++----------
>  drivers/clk/samsung/clk-exynos5x-subcmu.c |  1 +
>  drivers/soc/samsung/pm_domains.c          |  1 +
>  4 files changed, 38 insertions(+), 16 deletions(-)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

* Re: [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver
  2018-02-21 16:19       ` Krzysztof Kozlowski
@ 2018-02-22 12:22         ` Marek Szyprowski
  2018-02-23  7:49           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-22 12:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

Hi Krzysztof,

On 2018-02-21 17:19, Krzysztof Kozlowski wrote:
> On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Exynos5250/5420/5800 have only one clock constroller, but some of their
>> clock depends on respective power domains. Handling integration of clock
>> controller and power domain can be done using runtime PM feature of CCF
>> framework. This however needs a separate struct device for each power
>> domain. This patch adds such separate driver for group such clocks,
> "driver to group"?
>
>> which can be instantiated more than once, each time for a different
>> power domain.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/clk/samsung/clk-exynos5x-subcmu.c | 180 ++++++++++++++++++++++++++++++
> In some other places we call entire family "exynos5" so maybe let's
> follow the convention instead of "5x"? In the name of file and all the
> variables/names later?

Okay.

>>   drivers/clk/samsung/clk-exynos5x-subcmu.h |  30 +++++
>>   2 files changed, 210 insertions(+)
>>   create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c
>>   create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c b/drivers/clk/samsung/clk-exynos5x-subcmu.c
>> new file mode 100644
>> index 000000000000..9ff6d5d17f57
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
>> @@ -0,0 +1,180 @@
>> +/*
>> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
>> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
> Use SPDX identifier instead of license text.
>
>> + *
>> + * Common Clock Framework support for Exynos5x power-domain dependent
>> + * sub-CMUs
>> + */
>> +
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include "clk.h"
>> +#include "clk-exynos5x-subcmu.h"
>> +
>> +static struct samsung_clk_provider *ctx;
>> +static const struct samsung_5x_subcmu_info *cmu;
>> +static int nr_cmus;
> core_initcall here will register this and subcmu drivers. The probe of
> this driver will populate devices for subcmus. From next patch - the
> initcall of clock driver - will initialize the subcmus, also touching
> these statics.
>
> Is my understanding correct?
>
> Quite complicated init system... plus static variables. Are you sure
> there are no concurrency issues?

Everything is okay. exynos5420 clock driver is initialized very early
via OF_CLK_DECLARE() method. It calls

samsung_clk_subcmus_init() at the end, which passes the needed clk
context to this driver. Then there is core_init call, which registers
both platform drivers and then a bit later at arch_initcalls all
platform devices are populated from device tree. This triggers a probe
of exynos5x-clock platform driver, which in its probe creates child
platform devices for each supported power domain. Then those child
devices are probed by exynos5x-clock-subcmu platform driver, which
finally registers clocks and enables runtime pm for them.

I know this is a bit complex, but frankly I didn't find any other way
of handling clocks by both OF_CLK_DECLARE and platform device based
driver(s). Maybe I will put the above explanation in the comment
somewhere in this driver.

In theory exynos5x-clock platform driver is not really needed, as child
devices might have been created directly from OF_CLK_DECLARE()-based
code, but sadly it is called so early that it is not yet possible to
call any code related to platform bus (it is not yet initialized). The
good thing that all this is hidden inside this driver and there are no
external hacks needed elsewhere (like in DT) to get it working.

>> +
>> +static void samsung_ext_clk_save(void __iomem *base,
>> +                                   struct samsung_clk_ext_reg_dump *rd,
>> +                                   unsigned int num_regs)
>> +{
>> +       for (; num_regs > 0; --num_regs, ++rd) {
>> +               rd->save = readl(base + rd->offset);
>> +               writel((rd->save & ~rd->mask) | rd->value, base + rd->offset);
>> +               rd->save &= rd->mask;
>> +       }
>> +};
>> +
>> +static void samsung_ext_clk_restore(void __iomem *base,
>> +                                   struct samsung_clk_ext_reg_dump *rd,
>> +                                   unsigned int num_regs)
>> +{
>> +       for (; num_regs > 0; --num_regs, ++rd)
>> +               writel((readl(base + rd->offset) & ~rd->mask) | rd->save,
>> +                      base + rd->offset);
>> +}
>> +
>> +static int __maybe_unused exynos5x_clk_subcmu_suspend(struct device *dev)
>> +{
>> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&ctx->lock, flags);
>> +       samsung_ext_clk_save(ctx->reg_base, info->suspend_regs,
>> +                            info->nr_suspend_regs);
>> +       spin_unlock_irqrestore(&ctx->lock, flags);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __maybe_unused exynos5x_clk_subcmu_resume(struct device *dev)
>> +{
>> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&ctx->lock, flags);
>> +       samsung_ext_clk_restore(ctx->reg_base, info->suspend_regs,
>> +                               info->nr_suspend_regs);
>> +       spin_unlock_irqrestore(&ctx->lock, flags);
>> +
>> +       return 0;
>> +}
>> +
>> +static void samsung_clk_defer_gate(struct samsung_clk_provider *ctx,
>> +                         const struct samsung_gate_clock *list, int nr_clk)
>> +{
>> +       while (nr_clk--)
>> +               samsung_clk_add_lookup(ctx, ERR_PTR(-EPROBE_DEFER), list++->id);
>> +}
>> +
>> +void samsung_clk_subcmus_init(struct samsung_clk_provider *_ctx, int _nr_cmus,
>> +                             const struct samsung_5x_subcmu_info *_cmu)
>> +{
>> +       ctx = _ctx;
>> +       cmu = _cmu;
>> +       nr_cmus = _nr_cmus;
>> +
>> +       for (; _nr_cmus--; _cmu++) {
>> +               samsung_clk_defer_gate(ctx, _cmu->gate_clks, _cmu->nr_gate_clks);
>> +               samsung_ext_clk_save(ctx->reg_base, _cmu->suspend_regs,
>> +                                    _cmu->nr_suspend_regs);
>> +       }
>> +}
>> +
>> +static int __init exynos5x_clk_subcmu_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct samsung_5x_subcmu_info *info = dev_get_drvdata(dev);
>> +
>> +       pm_runtime_set_suspended(dev);
>> +       pm_runtime_enable(dev);
>> +       pm_runtime_get(dev);
>> +
>> +       ctx->dev = dev;
>> +       samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
>> +       samsung_clk_register_gate(ctx, info->gate_clks, info->nr_gate_clks);
>> +       ctx->dev = NULL;
>> +
>> +       pm_runtime_put_sync(dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct dev_pm_ops exynos5x_disp_pm_ops = {
>> +       SET_RUNTIME_PM_OPS(exynos5x_clk_subcmu_suspend,
>> +                          exynos5x_clk_subcmu_resume, NULL)
>> +       SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +                                    pm_runtime_force_resume)
>> +};
>> +
>> +static struct platform_driver exynos5x_clk_subcmu_driver __refdata = {
>> +       .driver = {
>> +               .name = "exynos5x-clock-subcmu",
>> +               .suppress_bind_attrs = true,
>> +               .pm = &exynos5x_disp_pm_ops,
>> +       },
>> +       .probe = exynos5x_clk_subcmu_probe,
>> +};
>> +
>> +static int __init exynos_5x_clk_register_subcmu(struct device *parent,
>> +                                           const struct samsung_5x_subcmu_info *info,
>> +                                               struct device_node *pd_node)
>> +{
>> +       struct of_phandle_args genpdspec = { .np = pd_node };
>> +       struct platform_device *pdev;
>> +
>> +       pdev = platform_device_alloc(info->pd_name, -1);
>> +       pdev->dev.parent = parent;
>> +       pdev->driver_override = "exynos5x-clock-subcmu";
>> +       platform_set_drvdata(pdev, (void *)info);
>> +       of_genpd_add_device(&genpdspec, &pdev->dev);
>> +       platform_device_add(pdev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __init exynos5x_clk_probe(struct platform_device *pdev)
>> +{
>> +       struct device_node *np;
>> +       const char *name;
>> +       int i;
>> +
>> +       for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
>> +               if (of_property_read_string(np, "label", &name) < 0)
>> +                       continue;
>> +               for (i = 0; i < nr_cmus; i++)
>> +                       if (strcmp(cmu[i].pd_name, name) == 0)
>> +                               exynos_5x_clk_register_subcmu(&pdev->dev,
>> +                                                             &cmu[i], np);
>> +       }
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id exynos5x_clk_of_match[] = {
>> +       { },
>> +};
>> +
>> +static struct platform_driver exynos5x_clk_driver __refdata = {
>> +       .driver = {
>> +               .name = "exynos5x-clock",
>> +               .of_match_table = exynos5x_clk_of_match,
>> +               .suppress_bind_attrs = true,
>> +       },
>> +       .probe = exynos5x_clk_probe,
>> +};
>> +
>> +static int __init exynos5x_clk_drv_init(void)
>> +{
>> +       platform_driver_register(&exynos5x_clk_driver);
>> +       platform_driver_register(&exynos5x_clk_subcmu_driver);
>> +       return 0;
>> +}
>> +core_initcall(exynos5x_clk_drv_init);
>> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.h b/drivers/clk/samsung/clk-exynos5x-subcmu.h
>> new file mode 100644
>> index 000000000000..b44c238e54fa
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
> SPDX as well plus a include guard.

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


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

* Re: [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver
  2018-02-22 12:22         ` Marek Szyprowski
@ 2018-02-23  7:49           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2018-02-23  7:49 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-clk, linux-samsung-soc, Sylwester Nawrocki, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

On Thu, Feb 22, 2018 at 1:22 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hi Krzysztof,
>
> On 2018-02-21 17:19, Krzysztof Kozlowski wrote:
>>
>> On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>>
>>> Exynos5250/5420/5800 have only one clock constroller, but some of their
>>> clock depends on respective power domains. Handling integration of clock
>>> controller and power domain can be done using runtime PM feature of CCF
>>> framework. This however needs a separate struct device for each power
>>> domain. This patch adds such separate driver for group such clocks,
>>
>> "driver to group"?
>>
>>> which can be instantiated more than once, each time for a different
>>> power domain.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/clk/samsung/clk-exynos5x-subcmu.c | 180
>>> ++++++++++++++++++++++++++++++
>>
>> In some other places we call entire family "exynos5" so maybe let's
>> follow the convention instead of "5x"? In the name of file and all the
>> variables/names later?
>
>
> Okay.
>
>
>>>   drivers/clk/samsung/clk-exynos5x-subcmu.h |  30 +++++
>>>   2 files changed, 210 insertions(+)
>>>   create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c
>>>   create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h
>>>
>>> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c
>>> b/drivers/clk/samsung/clk-exynos5x-subcmu.c
>>> new file mode 100644
>>> index 000000000000..9ff6d5d17f57
>>> --- /dev/null
>>> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c
>>> @@ -0,0 +1,180 @@
>>> +/*
>>> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
>>> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>
>> Use SPDX identifier instead of license text.
>>
>>> + *
>>> + * Common Clock Framework support for Exynos5x power-domain dependent
>>> + * sub-CMUs
>>> + */
>>> +
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_domain.h>
>>> +#include <linux/pm_runtime.h>
>>> +
>>> +#include "clk.h"
>>> +#include "clk-exynos5x-subcmu.h"
>>> +
>>> +static struct samsung_clk_provider *ctx;
>>> +static const struct samsung_5x_subcmu_info *cmu;
>>> +static int nr_cmus;
>>
>> core_initcall here will register this and subcmu drivers. The probe of
>> this driver will populate devices for subcmus. From next patch - the
>> initcall of clock driver - will initialize the subcmus, also touching
>> these statics.
>>
>> Is my understanding correct?
>>
>> Quite complicated init system... plus static variables. Are you sure
>> there are no concurrency issues?
>
>
> Everything is okay. exynos5420 clock driver is initialized very early
> via OF_CLK_DECLARE() method. It calls
>
> samsung_clk_subcmus_init() at the end, which passes the needed clk
> context to this driver. Then there is core_init call, which registers
> both platform drivers and then a bit later at arch_initcalls all
> platform devices are populated from device tree. This triggers a probe
> of exynos5x-clock platform driver, which in its probe creates child
> platform devices for each supported power domain. Then those child
> devices are probed by exynos5x-clock-subcmu platform driver, which
> finally registers clocks and enables runtime pm for them.
>
> I know this is a bit complex, but frankly I didn't find any other way
> of handling clocks by both OF_CLK_DECLARE and platform device based
> driver(s). Maybe I will put the above explanation in the comment
> somewhere in this driver.
>
> In theory exynos5x-clock platform driver is not really needed, as child
> devices might have been created directly from OF_CLK_DECLARE()-based
> code, but sadly it is called so early that it is not yet possible to
> call any code related to platform bus (it is not yet initialized). The
> good thing that all this is hidden inside this driver and there are no
> external hacks needed elsewhere (like in DT) to get it working.

Thanks for explanation - indeed it would be nice to see it somewhere
in the code. Also as you pointed, it is nice that everything is
driver-contained.

Best regards,
Krzysztof

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

* Re: [PATCH 0/6] Exynos5: cleanup clocks handling in power domains
       [not found]   ` <CANAwSgQirTqLMCCUWZNUKn3jVTCssjkD=H9Oe2R729K81xm2pg@mail.gmail.com>
@ 2018-02-26 13:09     ` Marek Szyprowski
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Szyprowski @ 2018-02-26 13:09 UTC (permalink / raw)
  To: Anand Moon
  Cc: open list:COMMON CLK FRAMEWORK, linux-samsung-soc,
	Sylwester Nawrocki, Chanwoo Choi, Inki Dae, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz

Hi Anand,

On 2018-02-22 06:42, Anand Moon wrote:
> On 21 February 2018 at 15:45, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> This patchset performs cleanup of the clock handling during Exynos power
>> domain power on/off sequences. This has been achieved by moving all clock
>> related operations from Exynos power domain driver to respective Exynos
>> clock controller drivers. Such change is possible after introducinng
>> runtime power-management in common clock framework.
>>
>> Another nice result of this cleanup is removal of deplock warning reported
>> in the following thread (the 'second issue'):
>> https://www.spinics.net/lists/linux-samsung-soc/msg61766.html
>>
>> [    5.932966] ======================================================
>> [    5.937199] usb 5-1: new high-speed USB device number 2 using xhci-hcd
>> [    5.939073] WARNING: possible circular locking dependency detected
>> [    5.939110] 4.15.0-rc8-next-20180116 #1121 Tainted: G        W
>> [    5.958143] ------------------------------------------------------
>> [    5.964299] kworker/0:1/59 is trying to acquire lock:
>> [    5.969304]  (&genpd->mlock){+.+.}, at: [<6abc3872>] genpd_runtime_resume+0x104/0x260
>> [    5.977155]
>> [    5.977155] but task is already holding lock:
>> [    5.982926]  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
>> [    5.990143]
>> [    5.990143] which lock already depends on the new lock.
>> [    5.990143]
>> [    5.998309]
>> [    5.998309] the existing dependency chain (in reverse order) is:
>> [    6.005739]
>> [    6.005739] -> #1 (prepare_lock){+.+.}:
>> [    6.011042]        mutex_lock_nested+0x1c/0x24
>> [    6.015419]        clk_prepare_lock+0x50/0xf8
>> [    6.019755]        clk_unprepare+0x1c/0x2c
>> [    6.023841]        exynos_pd_power+0x1a8/0x1e4
>> [    6.028246]        genpd_power_off+0x160/0x274
>> [    6.032664]        genpd_power_off_work_fn+0x2c/0x40
>> [    6.037630]        process_one_work+0x2d4/0x8f0
>> [    6.042104]        worker_thread+0x38/0x584
>> [    6.046268]        kthread+0x138/0x168
>> [    6.049981]        ret_from_fork+0x14/0x20
>> [    6.054044]          (null)
>> [    6.056794]
>> [    6.056794] -> #0 (&genpd->mlock){+.+.}:
>> [    6.062238]        __mutex_lock+0x7c/0xa68
>> [    6.066278]        mutex_lock_nested+0x1c/0x24
>> [    6.070703]        genpd_runtime_resume+0x104/0x260
>> [    6.075557]        __rpm_callback+0xc0/0x21c
>> [    6.079792]        rpm_callback+0x20/0x80
>> [    6.083774]        rpm_resume+0x558/0x7dc
>> [    6.087762]        __pm_runtime_resume+0x60/0x98
>> [    6.092367]        clk_core_prepare+0x44/0x490
>> [    6.096783]        clk_prepare+0x20/0x30
>> [    6.100674]        amba_get_enable_pclk+0x2c/0x60
>> [    6.105363]        amba_device_try_add+0x8c/0x20c
>> [    6.110041]        amba_deferred_retry_func+0x40/0xbc
>> [    6.115080]        process_one_work+0x2d4/0x8f0
>> [    6.119569]        worker_thread+0x38/0x584
>> [    6.123727]        kthread+0x138/0x168
>> [    6.127444]        ret_from_fork+0x14/0x20
>> [    6.131510]          (null)
>> [    6.134263]
>> [    6.134263] other info that might help us debug this:
>> [    6.134263]
>> [    6.142328]  Possible unsafe locking scenario:
>> [    6.142328]
>> [    6.148178]        CPU0                    CPU1
>> [    6.152656]        ----                    ----
>> [    6.157160]   lock(prepare_lock);
>> [    6.160439]                                lock(&genpd->mlock);
>> [    6.166365]                                lock(prepare_lock);
>> [    6.172168]   lock(&genpd->mlock);
>> [    6.175517]
>> [    6.175517]  *** DEADLOCK ***
>> [    6.175517]
>> [    6.181475] 4 locks held by kworker/0:1/59:
>> [    6.185580]  #0:  ((wq_completion)"events"){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
>> [    6.194407]  #1:  ((deferred_retry_work).work){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
>> [    6.203422]  #2:  (deferred_devices_lock){+.+.}, at: [<3e940c1f>] amba_deferred_retry_func+0x1c/0xbc
>> [    6.212522]  #3:  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
>> [    6.220128]
>> [    6.220128] stack backtrace:
>> [    6.224438] CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G        W        4.15.0-rc8-next-20180116 #1121
>> [    6.233757] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [    6.239791] Workqueue: events amba_deferred_retry_func
>> [    6.244929] [<c0111f90>] (unwind_backtrace) from [<c010e360>] (show_stack+0x10/0x14)
>> [    6.252670] [<c010e360>] (show_stack) from [<c0a19860>] (dump_stack+0x98/0xc4)
>> [    6.259877] [<c0a19860>] (dump_stack) from [<c0181478>] (print_circular_bug.constprop.17+0x210/0x32c)
>> [    6.269077] [<c0181478>] (print_circular_bug.constprop.17) from [<c01848f8>] (__lock_acquire+0x155c/0x1ac8)
>> [    6.278786] [<c01848f8>] (__lock_acquire) from [<c0185884>] (lock_acquire+0xe0/0x2bc)
>> [    6.286558] [<c0185884>] (lock_acquire) from [<c0a31788>] (__mutex_lock+0x7c/0xa68)
>> [    6.294181] [<c0a31788>] (__mutex_lock) from [<c0a32190>] (mutex_lock_nested+0x1c/0x24)
>> [    6.302161] [<c0a32190>] (mutex_lock_nested) from [<c05885dc>] (genpd_runtime_resume+0x104/0x260)
>> [    6.311008] [<c05885dc>] (genpd_runtime_resume) from [<c057c6c4>] (__rpm_callback+0xc0/0x21c)
>> [    6.319484] [<c057c6c4>] (__rpm_callback) from [<c057c840>] (rpm_callback+0x20/0x80)
>> [    6.327185] [<c057c840>] (rpm_callback) from [<c057c2a8>] (rpm_resume+0x558/0x7dc)
>> [    6.334721] [<c057c2a8>] (rpm_resume) from [<c057c58c>] (__pm_runtime_resume+0x60/0x98)
>> [    6.342706] [<c057c58c>] (__pm_runtime_resume) from [<c04b7f6c>] (clk_core_prepare+0x44/0x490)
>> [    6.351297] [<c04b7f6c>] (clk_core_prepare) from [<c04ba304>] (clk_prepare+0x20/0x30)
>> [    6.359081] [<c04ba304>] (clk_prepare) from [<c04b52f4>] (amba_get_enable_pclk+0x2c/0x60)
>> [    6.367229] [<c04b52f4>] (amba_get_enable_pclk) from [<c04b5510>] (amba_device_try_add+0x8c/0x20c)
>> [    6.376164] [<c04b5510>] (amba_device_try_add) from [<c04b56f8>] (amba_deferred_retry_func+0x40/0xbc)
>> [    6.385361] [<c04b56f8>] (amba_deferred_retry_func) from [<c01465b4>] (process_one_work+0x2d4/0x8f0)
>> [    6.394457] [<c01465b4>] (process_one_work) from [<c0147860>] (worker_thread+0x38/0x584)
>> [    6.402497] [<c0147860>] (worker_thread) from [<c014d794>] (kthread+0x138/0x168)
>> [    6.409852] [<c014d794>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
>>
>> This patchset affects Exynos5250 and Exynos5420/5422/5800 SoCs.
>>
>> Changes has been tested on Snow Chromebook (Exynos5250), Peach-Pit
>> Chromebook2 (Exynos5420) and Odroid XU3/XU4 (Exynos5422) boards with
>> latest linux-next kernel.
> I feel their is different CMU to support clk framework on Exynos platform.
> below are the some of the address of the CMU from the respective user manual.
>
> Exynos 4412:
> The address map of Exynos 4412 clock controller consists of six CMUs.
> They are, CMU_LEFTBUS, CMU_RIGHTBUS, CMU_TOP, CMU_DMC, CMU_CPU, and
> CMU_ISP. Each CMU uses an address space of 16 KB for SFRs.
> The internal structure of address space for each CMU is similar for all CMUs.
>
> 0x1003_4000         CMU_LEFTBUS
> 0x1003_8000         CMU_RIGHTBUS
> 0x1003_C000         CMU_TOP
> 0x1004_0000         CMU_DMC
> 0x1004_4000         CMU_CPU
>
> Exynos 5250:
> The address map of Exynos 5250 clock controller. There are nine CMUs
> in Exynos 5250 and
> each CMU uses 16 KB address space for SFRs. The nine CMUs are CMU_CPU,
> CMU_CORE, CMU_ACP,
> CMU_ISP, CMU_TOP, CMU_LEX, CMU_R0X, CMU_R1X, and CMU_CDREX. The
> internal structure of the
> address space for each CMU is similar to all CMUs.
>
> 0x1001_0000       CMU_CPU
> 0x1001_4000       CMU_CORE
> 0x1001_8000       CMU_ACP
> 0x1001_C000       CMU_ISP
> 0x1002_0000       CMU_TOP
> 0x1002_4000       CMU_LEX
> 0x1002_8000       CMU_R0X
> 0x1002_C000       CMU_R1X
> 0x1003_4000       CMU_CDREX
>
> Exynos5422:
> The address map of Exynos 5422 Clock Controller. Exynos 5422 provides
> for seven CMUs:
> CMU_CPU, CMU_KFC, CMU_CDREX, CMU_CPERI, CMU_G2D, CMU_ISP,  CMU_TOP
> Each CMU uses 16 KB address space for SFRs. The internal structure of
> the address space
> for each CMU is similar for all CMUs.
>
> 0x1001_0000         CMU_CPU
> 0x1001_4000         CMU_CPERI
> 0x1001_8000         CMU_G2D
> 0x1001_C000        CMU_ISP
> 0x1002_4000         CMU_TOP
> 0x1003_0000         CMU_CDREX
> 0x1003_4000         CMU_KFC
>
> I feel currently we have only one parent CMU for all the clock
> so is it possible to restructure the clk to support these CMU
> and also look into respective power-domain.
>
> I have read few parts of the documentation but dont have in-dept
> knowledge into this.
> Please correct me if my understating is wrong. I have limited
> knowledge on clk frame work.

Well, the main problem with 4412 and 5420/5422 is the fact that there is
no direct relation between each CMU and respective power domains.

For example display related clocks, which correspond to DISP power domain
are all mixed in CMU_TOP together with other non-display clocks. Same for
MFC, GSC, MSC and some ISP blocks. PERI, G2D, CPU and KFC all belong to
always-on power domains, so it is easier to have them handled together.

In short: there is no point in splitting clocks controller driver based on
the base address, as it simply gives no advantage at all.

 > ...

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


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

* Re: [PATCH 1/6] soc: samsung: pm_domains: Add blacklisting clock handling
  2018-02-22 10:54       ` Krzysztof Kozlowski
@ 2018-03-06 16:05         ` Sylwester Nawrocki
  0 siblings, 0 replies; 15+ messages in thread
From: Sylwester Nawrocki @ 2018-03-06 16:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Marek Szyprowski, linux-clk, linux-samsung-soc, Chanwoo Choi,
	Inki Dae, Bartlomiej Zolnierkiewicz

On 02/22/2018 11:54 AM, Krzysztof Kozlowski wrote:
> On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Handling of clock reparenting will be move to clock controller driver, so
>> add possibility to blacklist clock handling on systems, where the clock
>> controller already does all needed operations. This is needed to avoid
>> potential deadlock on clock reparenting during power domain on/off
>> procedure.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/soc/samsung/pm_domains.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Feel free to take it through samsung-clk tree. Just in case of
> possible conflicts - could you put it on separate branch please?

OK, it's applied on a separate branch now, please let me know when
you need a signed tag.

-- 
Thanks,
Sylwester

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

end of thread, other threads:[~2018-03-06 16:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20180221101532eucas1p24200cb65dba8209cd99dba5427c9a28d@eucas1p2.samsung.com>
2018-02-21 10:15 ` [PATCH 0/6] Exynos5: cleanup clocks handling in power domains Marek Szyprowski
     [not found]   ` <CGME20180221101533eucas1p234b1801844ce8fac633377d129323422@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 1/6] soc: samsung: pm_domains: Add blacklisting clock handling Marek Szyprowski
2018-02-22 10:54       ` Krzysztof Kozlowski
2018-03-06 16:05         ` Sylwester Nawrocki
     [not found]   ` <CGME20180221101533eucas1p27bcfd237d2c851402b3e99248fec5a6c@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 2/6] clk: samsung: Add Exynos5x sub-CMU clock driver Marek Szyprowski
2018-02-21 16:19       ` Krzysztof Kozlowski
2018-02-22 12:22         ` Marek Szyprowski
2018-02-23  7:49           ` Krzysztof Kozlowski
     [not found]   ` <CGME20180221101533eucas1p2c0fdc0b744b1e026906bd047507f5701@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 3/6] clk: samsung: exynos542x: Move PD-dependent clocks to Exynos5x sub-CMU driver Marek Szyprowski
2018-02-21 16:21       ` Krzysztof Kozlowski
     [not found]   ` <CGME20180221101534eucas1p24db3c1d049d9ecd0de9c76a10bb58041@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 4/6] clk: samsung: exynos5250: " Marek Szyprowski
2018-02-22 10:54       ` Krzysztof Kozlowski
     [not found]   ` <CGME20180221101534eucas1p29d832ffe11055241a39d79a8863845ad@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 5/6] soc: samsung: pm_domains: Deprecate support for clocks Marek Szyprowski
     [not found]   ` <CGME20180221101535eucas1p2e99fbf00fd97bb935b02d12e7c225da0@eucas1p2.samsung.com>
2018-02-21 10:15     ` [PATCH 6/6] ARM: dts: exynos: Remove obsolete clock properties from power domains Marek Szyprowski
     [not found]   ` <CANAwSgQirTqLMCCUWZNUKn3jVTCssjkD=H9Oe2R729K81xm2pg@mail.gmail.com>
2018-02-26 13:09     ` [PATCH 0/6] Exynos5: cleanup clocks handling in " 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.